[Mesa-dev] [PATCH] st/dri: fix winsys handle stride calculation for block formats

[Emil Velikov]

[+RobH since he's interested in plumbing similar/same formats into GBM]

Hi Philipp,

On 26 May 2016 at 13:00, Philipp Zabel <p.zabel at pengutronix.de> wrote:
> Am Donnerstag, den 26.05.2016, 12:43 +0100 schrieb Emil Velikov:
>> Hi gents,
>>
>> On 26 May 2016 at 11:28, Philipp Zabel <p.zabel at pengutronix.de> wrote:
>> > Hi Michel,
>> >
>> > Am Donnerstag, den 26.05.2016, 17:59 +0900 schrieb Michel Dänzer:
>> >> On 25.05.2016 22:20, Philipp Zabel wrote:
>> >> > This fixes the stride calculation for pipe formats with a block width
>> >> > larger than one.
>> >> >
>> >> > Signed-off-by: Philipp Zabel <p.zabel at pengutronix.de>
>> >> > ---
>> >> >  src/gallium/state_trackers/dri/dri2.c | 2 +-
>> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/src/gallium/state_trackers/dri/dri2.c b/src/gallium/state_trackers/dri/dri2.c
>> >> > index 0c84baf..c0b0d21 100644
>> >> > --- a/src/gallium/state_trackers/dri/dri2.c
>> >> > +++ b/src/gallium/state_trackers/dri/dri2.c
>> >> > @@ -804,7 +804,7 @@ dri2_create_image_from_name(__DRIscreen *_screen,
>> >> >     if (pf == PIPE_FORMAT_NONE)
>> >> >        return NULL;
>> >> >
>> >> > -   whandle.stride = pitch * util_format_get_blocksize(pf);
>> >> > +   whandle.stride = util_format_get_stride(pf, pitch);
>> >> >
>> >> >     return dri2_create_image_from_winsys(_screen, width, height, format,
>> >> >                                          &whandle, loaderPrivate);
>> >> >
>> >>
>> >> Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>
>> >>
>> >> Do you need somebody to push this patch for you?
>> >
>> > Yes, thank you.
>> >
>> Can we add a note if this fixes a real world case (on which driver
>> and/or format) ? Is it worth adding this patch in stable releases ?
>
> I encountered this when trying to import YUYV buffers via
> EGL_EXT_image_dma_buf_import into the (still out of tree) etnaviv
> gallium driver. Since I currently still have the following patch
> applied, I don't think this is a stable issue, at least regarding YUYV:
>
> ----------8<----------
> Subject: [PATCH] WIP: st/dri: Allow YUYV import
>
> Unclear whether this is the right way, but this allows to import
> dma-buffers with YUYV pixel format.
>
> Signed-off-by: Philipp Zabel <p.zabel at pengutronix.de>
>
> diff --git a/src/gallium/state_trackers/dri/dri2.c
> b/src/gallium/state_trackers/dri/dri2.c
> index e07389c..bad1d90 100644
> --- a/src/gallium/state_trackers/dri/dri2.c
> +++ b/src/gallium/state_trackers/dri/dri2.c
> @@ -70,6 +70,10 @@ static int convert_fourcc(int format, int
> *dri_components_p)
>        format = __DRI_IMAGE_FORMAT_XBGR8888;
>        dri_components = __DRI_IMAGE_COMPONENTS_RGB;
>        break;
> +   case __DRI_IMAGE_FOURCC_YUYV:
> +      format = __DRI_IMAGE_FOURCC_YUYV;
> +      dri_components = __DRI_IMAGE_COMPONENTS_Y_XUXV;
> +      break;
>     default:
>        return -1;
>     }
> @@ -118,6 +122,9 @@ static enum pipe_format dri2_format_to_pipe_format
> (int format)
>     case __DRI_IMAGE_FORMAT_ABGR8888:
>        pf = PIPE_FORMAT_RGBA8888_UNORM;
>        break;
> +   case __DRI_IMAGE_FOURCC_YUYV:
> +      pf = PIPE_FORMAT_YUYV;
> +      break;
>     default:
>        pf = PIPE_FORMAT_NONE;
>        break;
> ---------->8----------
>
> While I have your attention, should the above be handled by adding a
> __DRI_IMAGE_FORMAT_YUYV instead?
>
I think it's a good idea to have/add it. Internally drivers can handle
__DRI_IMAGE_{FOURCC,FORMAT}_YUYV in whichever way it seems best.
I'm not the most authoritative person to ask though - I'd imagine
Kristian Høgsberg may want to weight in. He added support for i965 for
the said fourcc not too long ago.

Is there a branch with your work so far ? I'd imagine that others
might be interested in taking a look.

Thanks
Emil