[Mesa-dev] [PATCH] egl/dri2: Allow modifiers to add FDs to imports

[Tapani Pälli]

On 08/08/2017 10:37 AM, Philipp Zabel wrote:
> On Tue, 2017-08-08 at 07:29 +0300, Tapani Pälli wrote:
>>
>> On 08/07/2017 03:05 PM, Philipp Zabel wrote:
>>> On Mon, 2017-07-31 at 18:35 +0100, Daniel Stone wrote:
>>>> When using dmabuf import, make sure that the modifier is actually
>>>> allowed to add planes to the base format, as implied by the comment.
>>>>
>>>> Signed-off-by: Daniel Stone <daniels at collabora.com>
>>>> ---
>>>>    src/egl/drivers/dri2/egl_dri2.c | 4 +++-
>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
>>>> index b73dcd72b6..76294897a5 100644
>>>> --- a/src/egl/drivers/dri2/egl_dri2.c
>>>> +++ b/src/egl/drivers/dri2/egl_dri2.c
>>>> @@ -2166,8 +2166,10 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
>>>>              *  this extension."
>>>>              */
>>>>             if (attrs->DMABufPlaneModifiersLo[i].IsPresent &&
>>>> -             attrs->DMABufPlaneModifiersHi[i].IsPresent)
>>>> +             attrs->DMABufPlaneModifiersHi[i].IsPresent) {
>>>> +            plane_n = i + 1;
>>>
>>> Since this increments plane_n, Should a check be added that the
>>> corresponding DMABufPlanFds[i] is present?
>>
>> Check for the fd is right above this check.
> 
> I see this right above:
> 
>        if (attrs->DMABufPlaneFds[i].IsPresent ||
>            attrs->DMABufPlaneOffsets[i].IsPresent ||
>            attrs->DMABufPlanePitches[i].IsPresent ||
>            attrs->DMABufPlaneModifiersLo[i].IsPresent ||
>            attrs->DMABufPlaneModifiersHi[i].IsPresent) {
> 
> If modifiers are present, this is always true, regardless of whether the
> fd is present.
> 
> The loop that checks for fd presence even before that only loops up to
> the number of planes determined by the non-modified fourcc.

Ah that is correct, my bad. It would be possible to swim through with 
having modifiers and not fd present.

// Tapani