Re: [PATCH i-g-t] tests/kms_plane: Allow planar-pixel-format-settings to pass on VKMS

["Sharma, Swati2" <swati2.sharma@intel.com>]

Hi Wang

No, patch LGTM

Reviewed-by: Swati Sharma <swati2.sharma@intel.com>

On 19-03-2026 01:46 pm, Renjun Wang wrote:
> Hi Sharma,
>
> The version 2 for this patch is send by Kamil Koneczny[1], and have CI 
> results. Do I need to send version 2 patch again?
>
>
> [1] 
> https://lore.kernel.org/igt-dev/20260216185231.164130-1-kamil.konieczny@linux.intel.com/
>
>
> Best Regards,
>
> On 2/20/26 04:09, Sharma, Swati2 wrote:
>> Hi Renjun,
>>
>> On 04-02-2026 05:59 pm, Kamil Konieczny wrote:
>>> Hi Renjun,
>>> On 2026-02-01 at 14:11:43 +0800, Renjun Wang wrote:
>>>> Set expected_rval to 0 for planar-pixel-format-settings tests when
>>>> running on VKMS driver. VKMS currently accepts odd width/height for
>>>> NV12, odd horizontal panning for NV12, and odd vertical panning for
>>>> P016 formats.
>>>>
>>>> Signed-off-by: Renjun Wang <renjunw0@foxmail.com>
>>>> ---
>>>>   tests/kms_plane.c | 25 +++++++++++++------------
>>>>   1 file changed, 13 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
>>>> index 21d80847c..57adfe199 100644
>>>> --- a/tests/kms_plane.c
>>>> +++ b/tests/kms_plane.c
>>>> @@ -1313,11 +1313,9 @@ static void test_planar_settings(data_t *data)
>>>>       int devid;
>>>>       int display_ver = -1;
>>>>       int rval;
>>>> +    bool is_intel = false;
>>>> +    bool is_vkms = false;
>>>>   -    /*
>>>> -     * If here is added non-intel tests below require will need to be
>>>> -     * changed to if(..)
>>>> -     */
>>>>       igt_require_f(data->display.is_atomic, "Atomic mode-set not 
>>>> supported\n");
>>>>       if (is_intel_device(data->drm_fd)) {
>>>>           igt_require_intel(data->drm_fd);
>>>> @@ -1325,6 +1323,9 @@ static void test_planar_settings(data_t *data)
>>>>           igt_require(intel_display_ver(devid) >= 9);
>>>>           display_ver = intel_display_ver(devid);
>>>>           igt_require(display_ver >= 9);
>>>> +        is_intel = true;
>>>> +    } else if (is_vkms_device(data->drm_fd)) {
>>>> +        is_vkms = true;
>>>>       }
>>>>         output = igt_get_single_output_for_pipe(&data->display, pipe);
>>>> @@ -1337,12 +1338,12 @@ static void test_planar_settings(data_t *data)
>>>>                     DRM_MODE_ATOMIC_ALLOW_MODESET,
>>>>                     NULL);
>>>>   -    /* test against intel_plane_check_src_coordinates() in i915 */
>>> I am not sure if removing this is proper, maybe move it just below
>>> before 'if((is_intel...'
>>> Same concern below, when you remove other i915 comments.
>>>
>>> +cc Karthik and Swati
>>> Cc: Karthik B S <karthik.b.s@intel.com>
>>> Cc: Swati Sharma <swati2.sharma@intel.com>
>>>
>>>> +    /* Test odd width for NV12 format */
>>> Do we need it here, when just below is DRM_FORMAT_NV12?
>>> Or is it 'odd width' that is worth pointing out?
>>> Same concern below.
>>>
>>> Overall looks good. Btw Karthik or Swati, please find someone
>>> from KMS team and send this for testing, I will ack it then.
>>>
>>> Regards,
>>> Kamil
>>
>> Agree with Kamil, lets keep existing comments as it is. Please add 
>> your changes on top of those.
>>
>> https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_14559/shards-all.html?testfilter=planar-pixel-format-settings 
>>
>>
>> https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_14559/shards-all.html?testfilter=planar-pixel-format-settings 
>>
>>
>> There isn't any regression on CI results, results are okay.
>>
>>>
>>>>       if (igt_plane_has_format_mod(primary, DRM_FORMAT_NV12,
>>>>                        DRM_FORMAT_MOD_LINEAR)) {
>>>>           int expected_rval = -EINVAL;
>>>>   -        if (display_ver >= 20)
>>>> +        if ((is_intel && display_ver >= 20) || is_vkms)
>>>>               expected_rval = 0;
>>>>             igt_create_fb(data->drm_fd, 257, 256,
>>>> @@ -1358,12 +1359,12 @@ static void test_planar_settings(data_t *data)
>>>>           igt_debug("Odd width NV12 framebuffer test skipped\n");
>>>>       }
>>>>   -    /* test against intel_plane_check_src_coordinates() in i915 */
>>>> +    /* Test odd height for NV12 format */
>>>>       if (igt_plane_has_format_mod(primary, DRM_FORMAT_NV12,
>>>>                        DRM_FORMAT_MOD_LINEAR)) {
>>>>           int expected_rval = -EINVAL;
>>>>   -        if (display_ver >= 20 && display_ver < 35)
>>>> +        if ((is_intel && display_ver >= 20 && display_ver < 35) || 
>>>> is_vkms)
>>>>               expected_rval = 0;
>>>>             igt_create_fb(data->drm_fd, 256, 257,
>>>> @@ -1378,13 +1379,12 @@ static void test_planar_settings(data_t *data)
>>>>       } else {
>>>>           igt_debug("Odd height NV12 framebuffer test skipped\n");
>>>>       }
>>>> -
>>>> -    /* test against intel_plane_check_src_coordinates() in i915 */
>>>> +    /*Test odd horizontal panning for NV12 format */
>>>>       if (igt_plane_has_format_mod(primary, DRM_FORMAT_NV12,
>>>>                        DRM_FORMAT_MOD_LINEAR)) {
>>>>           int expected_rval = -EINVAL;
>>>>   -        if (display_ver >= 35)
>>>> +        if ((is_intel && display_ver >= 35) || is_vkms)
>>>>               expected_rval = 0;
>>>>             igt_create_fb(data->drm_fd, 810, 590,
>>>> @@ -1401,11 +1401,12 @@ static void test_planar_settings(data_t *data)
>>>>           igt_debug("Odd horizontal pan NV12 framebuffer test 
>>>> skipped\n");
>>>>       }
>>>>   +    /* Test odd vertical panning for P016 format */
>>>>       if (igt_plane_has_format_mod(primary, DRM_FORMAT_P016,
>>>>                           DRM_FORMAT_MOD_LINEAR)) {
>>>>           int expected_rval = -EINVAL;
>>>>   -        if (display_ver >= 20 && display_ver < 35)
>>>> +        if ((is_intel && display_ver >= 20 && display_ver < 35) || 
>>>> is_vkms)
>>>>               expected_rval = 0;
>>>>             igt_create_color_fb(data->drm_fd, 256, 260,
>>>> -- 
>>>> 2.47.3
>>>>
>