[Piglit] [RFC] Trying to fix dmabuf/yuv tests

[Eric Anholt]

Rob Clark <robdclark at gmail.com> writes:

> On Tue, Sep 6, 2016 at 4:04 PM, Kristian Høgsberg <hoegsberg at gmail.com> wrote:
>> On Sun, Sep 4, 2016 at 9:34 AM, Rob Clark <robdclark at gmail.com> wrote:
>>> Ok, so the basic problem with the YUV tests is that they currently
>>> completely ignore driver/hw pitch requirements, since the code that
>>> allocates the buffer doesn't know the pixel format, only the 'cpp'.
>>>
>>> The yuv test creates a small 4x4 yuv eglimage.  If, say, the hardware
>>> requires the pitch to be aligned to, say, 32pixels, everything is fine
>>> for the Y plane, but the subsampled U/V or U+V plane has half as many
>>> pixels.  (This did help me catch a bug in driver, not rejecting the
>>> dmabuf import with invalid pitch, but that doesn't help to get the
>>> piglit tests running.)
>>>
>>> My original idea was that, since cpp values are never valid fourcc
>>> values, and visa versa, to avoid changing too much the other existing
>>> RGB tests by allowing to pass down either cpp or fourcc.  Especially
>>> since in the case of RGB we really don't care about, for example,
>>> ARGB vs BRGA.  Whereas for YUV formats, we really do want to know the
>>> exact format to copy the src data into the allocated buffer correctly.
>>> But this patch turned out to be big/intrusive enough that maybe just
>>> fixing up the other RGB tests to use fourcc everywhere might be ok.
>>>
>>> The other issue is that egl_image_for_dma_buf_fd() made assumptions
>>> about the offset/pitch of each plane, which were not valid.  (And,
>>> I believe, incorrect even for i965..  I'm not entirely sure why it
>>> worked at all.)  So I ended up needing to expose piglit_dma_buf and
>>> adding an array of per-plane offsets/pitches.
>>>
>>> Maybe this could be split up better or made a bit more clean.. not
>>> sure.
>>>
>>> Fwiw, this applies on top of anholt's gbm dmabuf backend.  And also
>>> needs the mesa patch I sent yesterday to add gbm R8/GR88 formats.
>>> ---
>>>  tests/spec/ext_image_dma_buf_import/refcount.c     |   6 +-
>>>  .../spec/ext_image_dma_buf_import/sample_common.c  |  70 ++++----
>>>  .../spec/ext_image_dma_buf_import/sample_common.h  |   5 +-
>>>  tests/spec/ext_image_dma_buf_import/sample_rgb.c   |   2 +-
>>>  tests/spec/ext_image_dma_buf_import/sample_yuv.c   |   2 +-
>>>  tests/util/piglit-framework-gl.c                   |   8 +-
>>>  tests/util/piglit-framework-gl.h                   |   2 +-
>>>  .../util/piglit-framework-gl/piglit_drm_dma_buf.c  | 176 ++++++++++++++++-----
>>>  .../util/piglit-framework-gl/piglit_drm_dma_buf.h  |  16 +-
>>>  .../util/piglit-framework-gl/piglit_gl_framework.h |   7 +-
>>>  10 files changed, 193 insertions(+), 101 deletions(-)
>>>

>> If we have the gbm eglimage path, why do we keep the intel specific code around?
>
> only plausible reason I could come up with is that gbm path depends on
> a fairly recent version of gbm to have the gbm_bo_map/unmap() bits..
> if that isn't an issue, we should drop the intel path.  Otherwise we
> should plan on eventually dropping intel path.

I didn't think I had the motivation to get functional equivalence to the
current Intel path in the first attempt, so I just got something working
for the rest of us to start with.  I think it would be great to switch
Intel to the gbm path as well if we could.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 800 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/piglit/attachments/20160906/622d91f8/attachment.sig>