[Mesa-dev] [v6 2/9] intel: do not create renderbuffers out of planar images

[Chad Versace]

On 06/02/2013 11:21 PM, Chad Versace wrote:
> On 05/30/2013 01:20 AM, Daniel Vetter wrote:
>> On Thu, May 30, 2013 at 9:49 AM, Pohjolainen, Topi
>> <topi.pohjolainen at intel.com> wrote:
>>> On Tue, May 28, 2013 at 01:33:18PM -0700, Eric Anholt wrote:
>>>> Topi Pohjolainen <topi.pohjolainen at intel.com> writes:
>>>>
>>>>> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
>>>>> ---
>>>>>   src/mesa/drivers/dri/intel/intel_fbo.c | 4 ++++
>>>>>   1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c b/src/mesa/drivers/dri/intel/intel_fbo.c
>>>>> index 69f8629..7ccbaa8 100644
>>>>> --- a/src/mesa/drivers/dri/intel/intel_fbo.c
>>>>> +++ b/src/mesa/drivers/dri/intel/intel_fbo.c
>>>>> @@ -280,6 +280,10 @@ intel_image_target_renderbuffer_storage(struct gl_context *ctx,
>>>>>      if (image == NULL)
>>>>>         return;
>>>>>
>>>>> +   /* Planar buffers are not supported as render targets. */
>>>>> +   if (image->planar_format && image->planar_format->nplanes > 1)
>>>>> +      return;
>>>>> +
>>>>>      /* __DRIimage is opaque to the core so it has to be checked here */
>>>>>      switch (image->format) {
>>>>>      case MESA_FORMAT_RGBA8888_REV:
>>>>
>>>> OK, this answers one question I had about what our support was going to
>>>> be.  But what about glEGLImageTargetTexture2DOES()?
>>>>
>>>> Are we only going to support planar textures with image_external?  My
>>>> thought is "yes".  How about on HSW with the YUV sampler support?  Are
>>>> we going to relayout the data in the incoming fds to a copy that can
>>>> support sampling from them, or are we going to reject incoming fds that
>>>> don't fit the required layout?  And if we decide to reject anything, are
>>>> we going to reject it at the or the dmabuf_import stage or the
>>>> image_external stage?
>>>
>>> I was also thinking that planar would be supported only by image_external, and I
>>> hadn't even thought about HSW yet.
>
> I think the same formats supported in image_external should also be supported
> by dma_buf_import. This would allow us to write Piglit tests for any supported
> image_external format.
>
> If the set of supported dma_buf_import formats is a subset of the image_external
> formats, then how do you propose we write tests (Piglit or non-Piglit) for the
> extra image_external formats?
>
> To avoid any implementation weirdness, though, I think we should prevent
> creation of 2D texture targets (GL_TEXTURE_2D) from dma_buf images, and
> permit only creation of external textures (GL_TEXTURE_EXTERNAL).

After considering this some more, I don't feel so strongly about it anymore.
That is, I don't really care if we do or do not support creation of 2D texture
targets from dma_buf images, as long as we do so only for non-planar dma_bufs.

>>> Clearly I haven't been thinking as far as you, I thought that images can be
>>> created relatively freely - it would be their uses that would fail if not
>>> supported. But this is wrong I guess. Once an image is created it should be
>>> guaranteed to be usable for quite a number of things.
>>>
>>> Having said that I realize that I don't most likely know enough about all the
>>> ways images can be used so that I could introduce proper constraints to the
>>> image creation logic.
>>
>> I guess we have two questions here: The first is deciding where we
>> fail the import process if the client api can't directly access the
>> data in the imported dma_buf. I think it's better to fail late since
>> doing all possible checks at egl image creation time might be ugly and
>> too restrictive. Otoh failing late will make it even harder for
>> applications to figure out what's possible.
>
> I think we should fail at image creation time if the driver is unable
> to texture from the given dma_buf configuration. If the image cannot
> be textured from, it's a useless brick, so why allow its creation?
>
> Validation of the image's texturability must be done at some time, most likely
> at either either time of image creation or of texture creation. I don't see how
> anything is gained by postponing the validation until time of
> texture creation, so let's just do it early so as to prevent the creation of
> bricked images.
>
>> The other questions is who will be responsible for copying. Doing any
>> kinds of transparent relayouting (e.g. when resampling a planar yuv
>> with oes image external to something a hw sampler could directly cope
>> with in case of a mismatch) doesn't sound too good: It'll break users
>> if they try to re-use dma_bufs/egl_images and the client api objects
>> created from them. So we might want to disallow any kind of egl image
>> orphaning and any other non-zero-copy conversion for images backed by
>> dma_bufs. Otoh this could get really ugly if we add a egl_image ->
>> dma_buf export extension.
>
> If we allow creation of a GL_TEXTURE_2D from dma_buf images, then the GL_OES_EGL_image
> spec requires orphaning under some situations. Here is issue #1 of that spec:
>
>      1.  What happens if an application tries to specify a new mipmap
>          level (or respecify an existing mipmap level) for a texture
>          object that was originally specified using
>          EGLImageTargetTexture2D (e.g., by subsequent calls to
>          {Copy}TexImage, GenerateMipmapOES, and/or setting the
>          texture's GENERATE_MIPMAP_SGIS parameter to TRUE) ?
>
>          RESOLVED:  If the application respecifies any properties of
>          an EGLImage target texture, the GL should allocate additional
>          memory for the respecified object, copying any data from
>          previously-specified levels (including those in the EGLImage
>          source).  The respecified texture object will not be an
>          EGLImage target, potentially orphaning the EGLImage.
>
> I'll say it again: I think we should prevent creation of 2D texture targets
> from dma_buf images, and permit only creation of external textures.
>
>> If we go with the "no orphaning for dma_buf backed egl_image" we could
>> make the copying somewhat more explicit with something like the egl
>> streams stuff. I haven't read the spec for that closely yet, but
>> having a solid zero-copy guarantee with making copying explicit sounds
>> good to me. Also, to better cope with interop issue we could guarantee
>> that every egl image which can be imported can at least be
>> hw-resampled into something useful. That way failing the
>> dma_buf->eg_image step still gives a useful piece of information to
>> users.
>>
>> Since this sounds like a spec question cc'ing all the people from the
>> original dma_buf_import spec discussions.
> S