Re: [PATCH] drm/i915/display: Workaround for odd panning for planar yuv

["Chauhan, Shekhar" <shekhar.chauhan@intel.com>]

On 10/18/2024 10:34, Garg, Nemesa wrote:
>
>> -----Original Message-----
>> From: Pottumuttu, Sai Teja <sai.teja.pottumuttu@intel.com>
>> Sent: Thursday, October 17, 2024 10:24 PM
>> To: Kandpal, Suraj <suraj.kandpal@intel.com>; Garg, Nemesa
>> <nemesa.garg@intel.com>; intel-xe@lists.freedesktop.org
>> Cc: Roper, Matthew D <matthew.d.roper@intel.com>
>> Subject: Re: [PATCH] drm/i915/display: Workaround for odd panning for planar
>> yuv
>>
>>
>> On 17-10-2024 13:35, Kandpal, Suraj wrote:
>>>> -----Original Message-----
>>>> From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of
>>>> Nemesa Garg
>>>> Sent: Wednesday, October 16, 2024 12:55 PM
>>>> To: intel-xe@lists.freedesktop.org
>>>> Cc: Garg, Nemesa <nemesa.garg@intel.com>
>>>> Subject: [PATCH] drm/i915/display: Workaround for odd panning for
>>>> planar yuv
>>>>
>>>> Disable the support for odd x pan for even xsize for NV12 format as
>>>> underrun issue is seen.
>> - Probably with the discussion and the latest update to the patch we should also
>> update the commit message to aptly reflect what are we doing here. (We can
>> remove even xsize part or maybe give more explanation on why even x size is not
>> being checked in the patch)
>>
>> - We should ideally be using the workaround lineage number in the commit
>> message.
>>
> Sure.
>> - Also, seems like NV12 is not the only planar format, should we check for a planar
>> format in general instead of NV12 specifically?
>> Cc: Matt Roper
>>
> In WA its mentioned that the issue is seen for YUV420 format so that's why I'm checking for NV12 format.
Could you please explain me:

1. Was this issue reproducible for Planar formats or Semi Planar formats 
(if they are different)?

2. Did we check it on only YUV_420 or all the planar formats (if the 
issue is only on Planar Formats). If it's only for YUV_420, then can we 
have commit title as "Workaround for odd panning of YUV_420..." or 
something like that. If not, then can we test it for the other formats 
as well?

3. Are YUV_420 and NV_12 formats the same, and is NV_12 a planar format 
or a semi_planar format? (drm/drm_fourcc.h makes me think otherwise, 
correct me if I'm wrong??)

-shekhar

CC: Matt Roper, Sai Teja

>
> Thanks,
> Nemesa
>
>> Thanks
>> Sai Teja
>>
>>>> WA: 16024459452
>>> This should be right above your signed-off-by The rest LGTM,
>>> Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
>>>
>>>> v2: Replace HSD with WA in commit message [Suraj]
>>>>       Modified the condition for handling odd panning
>>>>
>>>> v3: Simplified the condition for checking hsub
>>>>       Using older framework for wa as rev1[Jani]
>>>>
>>>> v4: Modify the condition for hsub [Sai Teja]
>>>>       Initialize hsub in else path [Dan]
>>>>
>>>> v5: Replace IS_LUNARLAKE with display version.
>>>>       Resolve nitpicks[Jani]
>>>>
>>>> v6: Replace -EINVAL with hsub [Suraj]
>>>>       Remove src_w check as not required
>>>>
>>>> Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/display/intel_atomic_plane.c | 5 +++++
>>>>    1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>>>> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>>>> index e979786aa5cf..2d7ca6e62926 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>>>> @@ -1031,6 +1031,11 @@ int intel_plane_check_src_coordinates(struct
>>>> intel_plane_state *plane_state)
>>>>    		 */
>>>>    		hsub = 1;
>>>>    		vsub = 1;
>>>> +
>>>> +		/* Wa_16023981245 */
>>>> +		if (DISPLAY_VER(i915) == 20 && fb->format->format ==
>>>> DRM_FORMAT_NV12 &&
>>>> +		    src_x % 2 != 0)
>>>> +			hsub = 2;
>>>>    	} else {
>>>>    		hsub = fb->format->hsub;
>>>>    		vsub = fb->format->vsub;
>>>> --
>>>> 2.25.1

-- 
-shekhar