[PATCH] modetest: consider supported formats before selecting a DRM plane

[Benjamin Gaignard]

Hi,

Do we have something more to do get this patch merge ?
Or do you see issue ?

Regards,
Benjamin

2014-04-04 16:36 GMT+02:00 Rob Clark <robdclark at gmail.com>:
> On Fri, Mar 28, 2014 at 6:15 AM, Fabien DESSENNE <fabien.dessenne at st.com> wrote:
>> This fixes an issue where the DRM planes do not support the same pixel
>> formats.
>> The current implementation selects a DRM plane without checking whether
>> the pixel format is supported or not. As a consequence modetest may try
>> to set up a plane not supporting the user request-format, which fails.
>> Modetest has to check the supported formats accross the plane list before
>> selecting a candidate.
>>
>> Signed-off-by: Fabien Dessenne <fabien.dessenne at st.com>
>
> Looks reasonable.  I suppose one drawback is that you could previously
> use modetest to test an error condition.  I'm not sure if anyone cares
> about that.. having a proper collection of tests for generic KMS APIs
> would of course be a better solution to that problem ;-)
>
> It could be a useful thing for linaro to look into intel-gpu-tools and
> see if it could be possible to refactor some of that into some
> cross-driver tests ;-)
>
> BR,
> -R
>
>> ---
>>  tests/modetest/modetest.c | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
>> index 4761c60..866ea82 100644
>> --- a/tests/modetest/modetest.c
>> +++ b/tests/modetest/modetest.c
>> @@ -951,7 +951,7 @@ static int set_plane(struct device *dev, struct plane_arg *p)
>>         int crtc_x, crtc_y, crtc_w, crtc_h;
>>         struct crtc *crtc = NULL;
>>         unsigned int pipe;
>> -       unsigned int i;
>> +       unsigned int i, j;
>>
>>         /* Find an unused plane which can be connected to our CRTC. Find the
>>          * CRTC index first, then iterate over available planes.
>> @@ -974,8 +974,11 @@ static int set_plane(struct device *dev, struct plane_arg *p)
>>                 if (!ovr)
>>                         continue;
>>
>> -               if ((ovr->possible_crtcs & (1 << pipe)) && !ovr->crtc_id)
>> -                       plane_id = ovr->plane_id;
>> +               if ((ovr->possible_crtcs & (1 << pipe)) && !ovr->crtc_id) {
>> +                       for (j = 0; j < ovr->count_formats; j++)
>> +                               if (!strncmp(p->format_str, (char *) &ovr->formats[j], 4))
>> +                                       plane_id = ovr->plane_id;
>> +               }
>>         }
>>
>>         if (!plane_id) {
>> --
>> 1.9.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Benjamin Gaignard

Graphic Working Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog