Bug 73512 - [clover] mesa.icd. should contain full path
Summary: [clover] mesa.icd. should contain full path
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Other (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: mesa-dev
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-11 22:14 UTC by Fabian Deutsch
Modified: 2015-08-01 19:10 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
0001-opencl-improved-auto-gen-.icd.patch (2.24 KB, patch)
2014-01-11 22:15 UTC, Igor Gnatenko
Details | Splinter Review
[PATCH v2] opencl: improved auto-gen .icd (2.24 KB, patch)
2014-01-11 22:42 UTC, Igor Gnatenko
Details | Splinter Review
[PATCH v2] opencl: improved auto-gen .icd (2.66 KB, patch)
2014-01-11 23:27 UTC, Igor Gnatenko
Details | Splinter Review
[PATCH v3] opencl: improved auto-gen .icd (2.71 KB, patch)
2014-01-13 19:11 UTC, Igor Gnatenko
Details | Splinter Review
[PATCH v4] opencl: improved auto-gen .icd (2.52 KB, patch)
2014-01-13 22:00 UTC, Igor Gnatenko
Details | Splinter Review
[PATCH] opencl: use versioned .so in mesa.icd (2.46 KB, patch)
2014-01-14 20:43 UTC, Igor Gnatenko
Details | Splinter Review
[PATCH] opencl: use versioned .so in mesa.icd (2.48 KB, patch)
2014-05-05 06:58 UTC, Igor Gnatenko
Details | Splinter Review
[PATCH rebased] opencl: use versioned .so in mesa.icd (2.49 KB, text/plain)
2014-05-25 14:25 UTC, Igor Gnatenko
Details

Description Fabian Deutsch 2014-01-11 22:14:07 UTC
The current mesa.icd file contains only the .so filename, but the .icd file should contain the complete path to the shared lib.
Comment 1 Igor Gnatenko 2014-01-11 22:15:26 UTC
Created attachment 91884 [details] [review]
0001-opencl-improved-auto-gen-.icd.patch

I've not checked this patch. Checking.
Comment 2 Igor Gnatenko 2014-01-11 22:42:01 UTC
Created attachment 91885 [details] [review]
[PATCH v2] opencl: improved auto-gen .icd

Ok. Well. I've tested patch and some updated.

--
v2: Use @OPENCL_VERSION@:0 for library
    replace /etc with @sysconfdir@ macros
Comment 3 Igor Gnatenko 2014-01-11 23:27:45 UTC
Created attachment 91886 [details] [review]
[PATCH v2] opencl: improved auto-gen .icd

oops. I've attached old patch. Sorry.
Comment 4 Tom Stellard 2014-01-13 17:46:01 UTC
According to the icd spec: http://www.khronos.org/registry/cl/extensions/khr/cl_khr_icd.txt

The vendors directory must go in /etc/OpenCL and also only the library name is included in the *.icd file, not the full path, so I don't think this patch is correct.

What problem does this patch fix?
Comment 5 Igor Gnatenko 2014-01-13 18:42:03 UTC
(In reply to comment #4)
> According to the icd spec:
> http://www.khronos.org/registry/cl/extensions/khr/cl_khr_icd.txt
> 
> The vendors directory must go in /etc/OpenCL and also only the library name
> is included in the *.icd file, not the full path, so I don't think this
> patch is correct.
> 
> What problem does this patch fix?
1. we're not installing .so to main package. we are installing it to -devel subpackage. So .icd should contain like .so.@LIBVER@
2. Fabian, Can you try to use liMesaOpenCL.so.1 in .icd file. What clinfo will do say ?
Comment 6 Fabian Deutsch 2014-01-13 18:57:07 UTC
Hey,

this can all be a result of me being uninformed (not knowing that only the library name is contained in the .icd file).

But I think that the .icd file is still not corect, as it contains only the unversioned library name libMesaOpenCL.so - which is - as Igor metions - not packaged in the main packages (only in devel subpackages).

So I'm not sure if the original icd file should contain the versioned library, or if we should do this downstream in Fedora.
Comment 7 Igor Gnatenko 2014-01-13 19:11:47 UTC
Created attachment 91973 [details] [review]
[PATCH v3] opencl: improved auto-gen .icd

v2: Use @OPENCL_VERSION@:0 for library
    replace /etc with @sysconfdir@ macros

v3: Drop libdir from icd, because libMesaOpenCL isn't private
Comment 8 Tom Stellard 2014-01-13 21:42:37 UTC
(In reply to comment #7)
> Created attachment 91973 [details] [review] [review]
> [PATCH v3] opencl: improved auto-gen .icd
> 
> v2: Use @OPENCL_VERSION@:0 for library
>     replace /etc with @sysconfdir@ macros
> 
> v3: Drop libdir from icd, because libMesaOpenCL isn't private

If we install the *.icd file to @sysconfdir@  and not /etc then standards compliant ICD loaders will not work with clover.  The way I interpret the spec, we have no choice, but to install it to /etc .  Why is it necessary to use @sysconfdir@ ?
Comment 9 Igor Gnatenko 2014-01-13 21:49:00 UTC
(In reply to comment #8)
> (In reply to comment #7)
> > Created attachment 91973 [details] [review] [review] [review]
> > [PATCH v3] opencl: improved auto-gen .icd
> > 
> > v2: Use @OPENCL_VERSION@:0 for library
> >     replace /etc with @sysconfdir@ macros
> > 
> > v3: Drop libdir from icd, because libMesaOpenCL isn't private
> 
> If we install the *.icd file to @sysconfdir@  and not /etc then standards
> compliant ICD loaders will not work with clover.  The way I interpret the
> spec, we have no choice, but to install it to /etc .  Why is it necessary to
> use @sysconfdir@ ?

why I can't install mesa in /usr/local or in /opt ? I think no problems there..

Should I update patch w/o/ this macros ?
Comment 10 Igor Gnatenko 2014-01-13 22:00:33 UTC
Created attachment 92004 [details] [review]
[PATCH v4] opencl: improved auto-gen .icd

v2: Use @OPENCL_VERSION@:0 for library
    replace /etc with @sysconfdir@ macros

v3: Drop libdir from icd, because libMesaOpenCL isn't private

v4: install ocl vendor always to /etc
Comment 11 Igor Gnatenko 2014-01-13 22:54:10 UTC
(In reply to comment #8)
> (In reply to comment #7)
> > Created attachment 91973 [details] [review] [review] [review]
> > [PATCH v3] opencl: improved auto-gen .icd
> > 
> > v2: Use @OPENCL_VERSION@:0 for library
> >     replace /etc with @sysconfdir@ macros
> > 
> > v3: Drop libdir from icd, because libMesaOpenCL isn't private
> 
> If we install the *.icd file to @sysconfdir@  and not /etc then standards
> compliant ICD loaders will not work with clover.  The way I interpret the
> spec, we have no choice, but to install it to /etc .  Why is it necessary to
> use @sysconfdir@ ?

Yes. I'm sorry. https://forge.imag.fr/plugins/scmgit/cgi-bin/gitweb.cgi?p=ocl-icd/ocl-icd.git;a=blob;f=ocl_icd_loader.c;h=ab419b2dccb82db6d632cae6dc86e5151a320c07;hb=HEAD#l52

Only /etc will work. Fixed. Patch here.
Comment 12 Igor Gnatenko 2014-01-14 20:43:45 UTC
Created attachment 92091 [details] [review]
[PATCH] opencl: use versioned .so in mesa.icd

We must have versioned library in mesa.icd, because ICD loader would
fail if the mesa-devel package wasn't installed.
Comment 13 Igor Gnatenko 2014-05-05 06:58:24 UTC
Created attachment 98450 [details] [review]
[PATCH] opencl: use versioned .so in mesa.icd

rebased for master
Comment 14 Igor Gnatenko 2014-05-25 14:25:55 UTC
Created attachment 99773 [details]
[PATCH rebased] opencl: use versioned .so in mesa.icd


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.