Re: [Mesa-dev] [Mesa-stable] [PATCH] st/dri: don't expose modifiers in EGL if the driver doesn't implement them

2017-10-02 Thread Emil Velikov 
On 27 September 2017 at 20:27, Juan A. Suarez Romero
 wrote:
> On Wed, 2017-09-27 at 18:09 +0100, Emil Velikov wrote:
>> On 27 September 2017 at 17:28, Marek Olšák  wrote:
>> > On Wed, Sep 27, 2017 at 6:22 PM, Emil Velikov  
>> > wrote:
>> > > On 27 September 2017 at 16:00, Daniel Stone  wrote:
>> > > > Hi Marek,
>> > > >
>> > > > On 27 September 2017 at 15:55, Marek Olšák  wrote:
>> > > > > if (dmabuf_ret && dmabuf_ret->val.val_bool) {
>> > > > >uint64_t cap;
>> > > > >
>> > > > >if (drmGetCap(sPriv->fd, DRM_CAP_PRIME, ) == 0 &&
>> > > > >(cap & DRM_PRIME_CAP_IMPORT)) {
>> > > > >   dri2ImageExtension.createImageFromFds = dri2_from_fds;
>> > > > >   dri2ImageExtension.createImageFromDmaBufs = 
>> > > > > dri2_from_dma_bufs;
>> > > > >   dri2ImageExtension.createImageFromDmaBufs2 = 
>> > > > > dri2_from_dma_bufs2;
>> > > > >   dri2ImageExtension.queryDmaBufFormats = 
>> > > > > dri2_query_dma_buf_formats;
>> > > > > - dri2ImageExtension.queryDmaBufModifiers =
>> > > > > -dri2_query_dma_buf_modifiers;
>> > > > > + if (pscreen->query_dmabuf_modifiers) {
>> > > > > +dri2ImageExtension.queryDmaBufModifiers =
>> > > > > +   dri2_query_dma_buf_modifiers;
>> > > > > + }
>> > > >
>> > > > This should also not expose queryDmaBufFormats, since that is also
>> > > > part of EGL_EXT_image_dma_buf_import_modifiers, which is pretty
>> > > > useless without modifiers.
>> > > >
>> > >
>> > > True, it's useless. Suggestion makes the code a bit confusing though.
>> > > After all EGL already checks that all the entry points are present
>> > > before advertising the extension.
>> > >
>> > > Either way, I think we want this in stable, right?
>> >
>> > Sorry too late, I pushed it.
>> >
>>
>> No need to apologise.
>>
>> > I don't know if stable is affected.
>> >
>>
>> You're right - code was introduced around commit
>> f84bb6a9d91521de6da4c3d1ddd8de456761efaa.
>> The latter of which landed in Mesa 17.2.0-devel
>>
>
> Not sure if I'm understanding correctly. Do you mean we don't need this
> patch in stable?
>
Hmm I misread the version as "17.3.0-devel" ...
Patch is a simple NULL check so it won't hurt to have in 17.2.x

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [Mesa-stable] [PATCH] st/dri: don't expose modifiers in EGL if the driver doesn't implement them

2017-09-27 Thread Juan A. Suarez Romero 
On Wed, 2017-09-27 at 18:09 +0100, Emil Velikov wrote:
> On 27 September 2017 at 17:28, Marek Olšák  wrote:
> > On Wed, Sep 27, 2017 at 6:22 PM, Emil Velikov  
> > wrote:
> > > On 27 September 2017 at 16:00, Daniel Stone  wrote:
> > > > Hi Marek,
> > > > 
> > > > On 27 September 2017 at 15:55, Marek Olšák  wrote:
> > > > > if (dmabuf_ret && dmabuf_ret->val.val_bool) {
> > > > >uint64_t cap;
> > > > > 
> > > > >if (drmGetCap(sPriv->fd, DRM_CAP_PRIME, ) == 0 &&
> > > > >(cap & DRM_PRIME_CAP_IMPORT)) {
> > > > >   dri2ImageExtension.createImageFromFds = dri2_from_fds;
> > > > >   dri2ImageExtension.createImageFromDmaBufs = 
> > > > > dri2_from_dma_bufs;
> > > > >   dri2ImageExtension.createImageFromDmaBufs2 = 
> > > > > dri2_from_dma_bufs2;
> > > > >   dri2ImageExtension.queryDmaBufFormats = 
> > > > > dri2_query_dma_buf_formats;
> > > > > - dri2ImageExtension.queryDmaBufModifiers =
> > > > > -dri2_query_dma_buf_modifiers;
> > > > > + if (pscreen->query_dmabuf_modifiers) {
> > > > > +dri2ImageExtension.queryDmaBufModifiers =
> > > > > +   dri2_query_dma_buf_modifiers;
> > > > > + }
> > > > 
> > > > This should also not expose queryDmaBufFormats, since that is also
> > > > part of EGL_EXT_image_dma_buf_import_modifiers, which is pretty
> > > > useless without modifiers.
> > > > 
> > > 
> > > True, it's useless. Suggestion makes the code a bit confusing though.
> > > After all EGL already checks that all the entry points are present
> > > before advertising the extension.
> > > 
> > > Either way, I think we want this in stable, right?
> > 
> > Sorry too late, I pushed it.
> > 
> 
> No need to apologise.
> 
> > I don't know if stable is affected.
> > 
> 
> You're right - code was introduced around commit
> f84bb6a9d91521de6da4c3d1ddd8de456761efaa.
> The latter of which landed in Mesa 17.2.0-devel
> 

Not sure if I'm understanding correctly. Do you mean we don't need this
patch in stable?

J.A.

> -Emil
> ___
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev