drm/i915/display: Workaround for odd panning for planar yuv

Commit Message

Nemesa Garg Sept. 30, 2024, 11:23 a.m. UTC

Disable the support for odd x pan for even xsize for NV12 format as underrun
issue is seen.

WA: 16024459452

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]

Signed-off-by: Nemesa Garg <[e-mail]>
---
 drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Kandpal, Suraj Oct. 15, 2024, 9:02 a.m. UTC | #1

> -----Original Message-----
> From: Intel-gfx <[e-mail]> On Behalf Of
> Nemesa Garg
> Sent: Monday, September 30, 2024 4:54 PM
> To: [e-mail]
> Cc: Garg, Nemesa <[e-mail]>
> 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.
> 
> WA: 16024459452
> 
> 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]
> 
> Signed-off-by: Nemesa Garg <[e-mail]>
> ---
>  drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8 ++++++++
>  1 file changed, 8 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..e3401a4f7992 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -1029,6 +1029,14 @@ int intel_plane_check_src_coordinates(struct
> intel_plane_state *plane_state)
>  		 * This allows NV12 and P0xx formats to have odd size
> and/or odd
>  		 * source coordinates on DISPLAY_VER(i915) >= 20
>  		 */
> +		/*
> +		 *  Wa_16023981245 for display version 20.
> +		 *  Do not support odd x-panning for even xsize for NV12.

Only mention WA no here the rest anyone can refer to by going and checking the HSD 

> +		 */
> +		if (DISPLAY_VER(i915) == 20 && fb->format->format ==
> DRM_FORMAT_NV12 &&
> +		    src_x % 2 != 0 && src_w % 2 == 0)
> +			return -EINVAL;

Also rather than return -EINVAL here we set hsub =2 and vsub =1 this should make sure odd panning is disabled
When we have even size since we only need to disable odd panning according to hsd not even panning this return may end
stop panning for even sizes all together.

Regards,
Suraj Kandpal
> +
>  		hsub = 1;
>  		vsub = 1;
>  	} else {
> --
> 2.25.1

Kandpal, Suraj Oct. 15, 2024, 9:25 a.m. UTC | #2

> -----Original Message-----
> From: Intel-gfx <[e-mail]> On Behalf Of
> Nemesa Garg
> Sent: Monday, September 30, 2024 4:54 PM
> To: [e-mail]
> Cc: Garg, Nemesa <[e-mail]>
> 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.
> 
> WA: 16024459452
> 
> 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]
> 
> Signed-off-by: Nemesa Garg <[e-mail]>
> ---
>  drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8 ++++++++
>  1 file changed, 8 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..e3401a4f7992 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -1029,6 +1029,14 @@ int intel_plane_check_src_coordinates(struct
> intel_plane_state *plane_state)
>  		 * This allows NV12 and P0xx formats to have odd size
> and/or odd
>  		 * source coordinates on DISPLAY_VER(i915) >= 20
>  		 */
> +		/*
> +		 *  Wa_16023981245 for display version 20.
> +		 *  Do not support odd x-panning for even xsize for NV12.
> +		 */
> +		if (DISPLAY_VER(i915) == 20 && fb->format->format ==
> DRM_FORMAT_NV12 &&
> +		    src_x % 2 != 0 && src_w % 2 == 0)

Also one more issue here according to HSD " Odd Pan position  + Even plane size for YVU420 ..... SW decision is to not allow Odd Pan X position"
Which would mean you need to check src_w and src_h instead of src_x to check even plane.

Regards,
Suraj Kandpal
> +			return -EINVAL;
> +
>  		hsub = 1;
>  		vsub = 1;
>  	} else {
> --
> 2.25.1

Nemesa Garg Oct. 15, 2024, 1:21 p.m. UTC | #3

> -----Original Message-----
> From: Kandpal, Suraj <[e-mail]>
> Sent: Tuesday, October 15, 2024 2:33 PM
> To: Garg, Nemesa <[e-mail]>; [e-mail]
> Cc: Garg, Nemesa <[e-mail]>
> Subject: RE: [PATCH] drm/i915/display: Workaround for odd panning for planar
> yuv
> 
> 
> 
> > -----Original Message-----
> > From: Intel-gfx <[e-mail]> On Behalf Of
> > Nemesa Garg
> > Sent: Monday, September 30, 2024 4:54 PM
> > To: [e-mail]
> > Cc: Garg, Nemesa <[e-mail]>
> > 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.
> >
> > WA: 16024459452
> >
> > 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]
> >
> > Signed-off-by: Nemesa Garg <[e-mail]>
> > ---
> >  drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8 ++++++++
> >  1 file changed, 8 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..e3401a4f7992 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > @@ -1029,6 +1029,14 @@ int intel_plane_check_src_coordinates(struct
> > intel_plane_state *plane_state)
> >  		 * This allows NV12 and P0xx formats to have odd size and/or
> odd
> >  		 * source coordinates on DISPLAY_VER(i915) >= 20
> >  		 */
> > +		/*
> > +		 *  Wa_16023981245 for display version 20.
> > +		 *  Do not support odd x-panning for even xsize for NV12.
> 
> Only mention WA no here the rest anyone can refer to by going and checking the
> HSD
Sure..will do.
> 
> > +		 */
> > +		if (DISPLAY_VER(i915) == 20 && fb->format->format ==
> > DRM_FORMAT_NV12 &&
> > +		    src_x % 2 != 0 && src_w % 2 == 0)
> > +			return -EINVAL;
> 
> Also rather than return -EINVAL here we set hsub =2 and vsub =1 this should
> make sure odd panning is disabled When we have even size since we only need to
> disable odd panning according to hsd not even panning this return may end stop
> panning for even sizes all together.
> 
Even if I do hsub=2 then also it will return -EINVAL as src_x will be odd and src_x % hsub will be 1 which will return -EINVAL.
Even panning doesn't get disable that's why I have added the check src_x % 2 != 0 to make sure that position in not even
and src_w % 2 to check for even size.

Regards,
Nemesa

> Regards,
> Suraj Kandpal
> > +
> >  		hsub = 1;
> >  		vsub = 1;
> >  	} else {
> > --
> > 2.25.1

Nemesa Garg Oct. 15, 2024, 1:23 p.m. UTC | #4

> -----Original Message-----
> From: Kandpal, Suraj <[e-mail]>
> Sent: Tuesday, October 15, 2024 2:55 PM
> To: Garg, Nemesa <[e-mail]>; [e-mail]
> Cc: Garg, Nemesa <[e-mail]>
> Subject: RE: [PATCH] drm/i915/display: Workaround for odd panning for planar
> yuv
> 
> 
> 
> > -----Original Message-----
> > From: Intel-gfx <[e-mail]> On Behalf Of
> > Nemesa Garg
> > Sent: Monday, September 30, 2024 4:54 PM
> > To: [e-mail]
> > Cc: Garg, Nemesa <[e-mail]>
> > 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.
> >
> > WA: 16024459452
> >
> > 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]
> >
> > Signed-off-by: Nemesa Garg <[e-mail]>
> > ---
> >  drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8 ++++++++
> >  1 file changed, 8 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..e3401a4f7992 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > @@ -1029,6 +1029,14 @@ int intel_plane_check_src_coordinates(struct
> > intel_plane_state *plane_state)
> >  		 * This allows NV12 and P0xx formats to have odd size and/or
> odd
> >  		 * source coordinates on DISPLAY_VER(i915) >= 20
> >  		 */
> > +		/*
> > +		 *  Wa_16023981245 for display version 20.
> > +		 *  Do not support odd x-panning for even xsize for NV12.
> > +		 */
> > +		if (DISPLAY_VER(i915) == 20 && fb->format->format ==
> > DRM_FORMAT_NV12 &&
> > +		    src_x % 2 != 0 && src_w % 2 == 0)
> 
> Also one more issue here according to HSD " Odd Pan position  + Even plane size
> for YVU420 ..... SW decision is to not allow Odd Pan X position"
> Which would mean you need to check src_w and src_h instead of src_x to check
> even plane.
> 
As we need to disable odd panning in x direction so I think src_w will suffice, no need to check src_h as we are not 
changing anything in y -direction.

Regards,
Nemesa

> Regards,
> Suraj Kandpal
> > +			return -EINVAL;
> > +
> >  		hsub = 1;
> >  		vsub = 1;
> >  	} else {
> > --
> > 2.25.1

Kandpal, Suraj Oct. 15, 2024, 2:47 p.m. UTC | #5

> -----Original Message-----
> From: Garg, Nemesa <[e-mail]>
> Sent: Tuesday, October 15, 2024 6:54 PM
> To: Kandpal, Suraj <[e-mail]>; intel-
> [e-mail]
> Subject: RE: [PATCH] drm/i915/display: Workaround for odd panning for
> planar yuv
> 
> 
> 
> > -----Original Message-----
> > From: Kandpal, Suraj <[e-mail]>
> > Sent: Tuesday, October 15, 2024 2:55 PM
> > To: Garg, Nemesa <[e-mail]>;
> > [e-mail]
> > Cc: Garg, Nemesa <[e-mail]>
> > Subject: RE: [PATCH] drm/i915/display: Workaround for odd panning for
> > planar yuv
> >
> >
> >
> > > -----Original Message-----
> > > From: Intel-gfx <[e-mail]> On Behalf
> > > Of Nemesa Garg
> > > Sent: Monday, September 30, 2024 4:54 PM
> > > To: [e-mail]
> > > Cc: Garg, Nemesa <[e-mail]>
> > > 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.
> > >
> > > WA: 16024459452
> > >
> > > 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]
> > >
> > > Signed-off-by: Nemesa Garg <[e-mail]>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8 ++++++++
> > >  1 file changed, 8 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..e3401a4f7992 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > @@ -1029,6 +1029,14 @@ int intel_plane_check_src_coordinates(struct
> > > intel_plane_state *plane_state)
> > >  		 * This allows NV12 and P0xx formats to have odd size
> and/or
> > odd
> > >  		 * source coordinates on DISPLAY_VER(i915) >= 20
> > >  		 */
> > > +		/*
> > > +		 *  Wa_16023981245 for display version 20.
> > > +		 *  Do not support odd x-panning for even xsize for NV12.
> > > +		 */
> > > +		if (DISPLAY_VER(i915) == 20 && fb->format->format ==
> > > DRM_FORMAT_NV12 &&
> > > +		    src_x % 2 != 0 && src_w % 2 == 0)
> >
> > Also one more issue here according to HSD " Odd Pan position  + Even
> > plane size for YVU420 ..... SW decision is to not allow Odd Pan X position"
> > Which would mean you need to check src_w and src_h instead of src_x to
> > check even plane.
> >
> As we need to disable odd panning in x direction so I think src_w will
> suffice, no need to check src_h as we are not changing anything in y -
> direction.

No it asks us to implement the WA for an even plane size which would mean checking for
Both src_w and src_h

Regards,
Suraj Kandpal

> 
> Regards,
> Nemesa
> 
> > Regards,
> > Suraj Kandpal
> > > +			return -EINVAL;
> > > +
> > >  		hsub = 1;
> > >  		vsub = 1;
> > >  	} else {
> > > --
> > > 2.25.1

Kandpal, Suraj Oct. 15, 2024, 2:53 p.m. UTC | #6

> -----Original Message-----
> From: Garg, Nemesa <[e-mail]>
> Sent: Tuesday, October 15, 2024 6:52 PM
> To: Kandpal, Suraj <[e-mail]>; intel-
> [e-mail]
> Subject: RE: [PATCH] drm/i915/display: Workaround for odd panning for
> planar yuv
> 
> 
> 
> > -----Original Message-----
> > From: Kandpal, Suraj <[e-mail]>
> > Sent: Tuesday, October 15, 2024 2:33 PM
> > To: Garg, Nemesa <[e-mail]>;
> > [e-mail]
> > Cc: Garg, Nemesa <[e-mail]>
> > Subject: RE: [PATCH] drm/i915/display: Workaround for odd panning for
> > planar yuv
> >
> >
> >
> > > -----Original Message-----
> > > From: Intel-gfx <[e-mail]> On Behalf
> > > Of Nemesa Garg
> > > Sent: Monday, September 30, 2024 4:54 PM
> > > To: [e-mail]
> > > Cc: Garg, Nemesa <[e-mail]>
> > > 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.
> > >
> > > WA: 16024459452
> > >
> > > 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]
> > >
> > > Signed-off-by: Nemesa Garg <[e-mail]>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8 ++++++++
> > >  1 file changed, 8 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..e3401a4f7992 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > @@ -1029,6 +1029,14 @@ int intel_plane_check_src_coordinates(struct
> > > intel_plane_state *plane_state)
> > >  		 * This allows NV12 and P0xx formats to have odd size
> and/or
> > odd
> > >  		 * source coordinates on DISPLAY_VER(i915) >= 20
> > >  		 */
> > > +		/*
> > > +		 *  Wa_16023981245 for display version 20.
> > > +		 *  Do not support odd x-panning for even xsize for NV12.
> >
> > Only mention WA no here the rest anyone can refer to by going and
> > checking the HSD
> Sure..will do.
> >
> > > +		 */
> > > +		if (DISPLAY_VER(i915) == 20 && fb->format->format ==
> > > DRM_FORMAT_NV12 &&
> > > +		    src_x % 2 != 0 && src_w % 2 == 0)
> > > +			return -EINVAL;
> >
> > Also rather than return -EINVAL here we set hsub =2 and vsub =1 this
> > should make sure odd panning is disabled When we have even size since
> > we only need to disable odd panning according to hsd not even panning
> > this return may end stop panning for even sizes all together.
> >
> Even if I do hsub=2 then also it will return -EINVAL as src_x will be odd and
> src_x % hsub will be 1 which will return -EINVAL.
> Even panning doesn't get disable that's why I have added the check src_x %
> 2 != 0 to make sure that position in not even and src_w % 2 to check for
> even size.


That will be fine let the code later that already exists take care of that since we don't
Want to abruptly send an -EINVAL in a code block where we were assigning variables. 
When it fails later as you said it will with a clean debug message too.
Secondly when you do return -EINVAL for src_x % 2 != 0 it ends up stopping panning in y direction
If the current src_x is odd but panning is being done only in y direction it will return -EINVAL
Hence assigning hsub and vsub here make a lot more sense.
We can remove the src_x check and keep src_w and add src_h check

Regards,
Suraj Kandpal

> 
> Regards,
> Nemesa
> 
> > Regards,
> > Suraj Kandpal
> > > +
> > >  		hsub = 1;
> > >  		vsub = 1;
> > >  	} else {
> > > --
> > > 2.25.1

Nemesa Garg Oct. 15, 2024, 4:23 p.m. UTC | #7

> -----Original Message-----
> From: Kandpal, Suraj <[e-mail]>
> Sent: Tuesday, October 15, 2024 8:24 PM
> To: Garg, Nemesa <[e-mail]>; [e-mail]
> Subject: RE: [PATCH] drm/i915/display: Workaround for odd panning for planar
> yuv
> 
> 
> 
> > -----Original Message-----
> > From: Garg, Nemesa <[e-mail]>
> > Sent: Tuesday, October 15, 2024 6:52 PM
> > To: Kandpal, Suraj <[e-mail]>; intel-
> > [e-mail]
> > Subject: RE: [PATCH] drm/i915/display: Workaround for odd panning for
> > planar yuv
> >
> >
> >
> > > -----Original Message-----
> > > From: Kandpal, Suraj <[e-mail]>
> > > Sent: Tuesday, October 15, 2024 2:33 PM
> > > To: Garg, Nemesa <[e-mail]>;
> > > [e-mail]
> > > Cc: Garg, Nemesa <[e-mail]>
> > > Subject: RE: [PATCH] drm/i915/display: Workaround for odd panning
> > > for planar yuv
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Intel-gfx <[e-mail]> On
> > > > Behalf Of Nemesa Garg
> > > > Sent: Monday, September 30, 2024 4:54 PM
> > > > To: [e-mail]
> > > > Cc: Garg, Nemesa <[e-mail]>
> > > > 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.
> > > >
> > > > WA: 16024459452
> > > >
> > > > 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]
> > > >
> > > > Signed-off-by: Nemesa Garg <[e-mail]>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8 ++++++++
> > > >  1 file changed, 8 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..e3401a4f7992 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > > @@ -1029,6 +1029,14 @@ int
> > > > intel_plane_check_src_coordinates(struct
> > > > intel_plane_state *plane_state)
> > > >  		 * This allows NV12 and P0xx formats to have odd size
> > and/or
> > > odd
> > > >  		 * source coordinates on DISPLAY_VER(i915) >= 20
> > > >  		 */
> > > > +		/*
> > > > +		 *  Wa_16023981245 for display version 20.
> > > > +		 *  Do not support odd x-panning for even xsize for NV12.
> > >
> > > Only mention WA no here the rest anyone can refer to by going and
> > > checking the HSD
> > Sure..will do.
> > >
> > > > +		 */
> > > > +		if (DISPLAY_VER(i915) == 20 && fb->format->format ==
> > > > DRM_FORMAT_NV12 &&
> > > > +		    src_x % 2 != 0 && src_w % 2 == 0)
> > > > +			return -EINVAL;
> > >
> > > Also rather than return -EINVAL here we set hsub =2 and vsub =1 this
> > > should make sure odd panning is disabled When we have even size
> > > since we only need to disable odd panning according to hsd not even
> > > panning this return may end stop panning for even sizes all together.
> > >
> > Even if I do hsub=2 then also it will return -EINVAL as src_x will be
> > odd and src_x % hsub will be 1 which will return -EINVAL.
> > Even panning doesn't get disable that's why I have added the check
> > src_x %
> > 2 != 0 to make sure that position in not even and src_w % 2 to check
> > for even size.
> 
> 
> That will be fine let the code later that already exists take care of that since we
> don't Want to abruptly send an -EINVAL in a code block where we were assigning
> variables.
> When it fails later as you said it will with a clean debug message too.
> Secondly when you do return -EINVAL for src_x % 2 != 0 it ends up stopping
> panning in y direction If the current src_x is odd but panning is being done only in
> y direction it will return -EINVAL Hence assigning hsub and vsub here make a lot
> more sense.
> We can remove the src_x check and keep src_w and add src_h check
> 
But if I remove  src_x % 2 != 0 then above condition will get true for even positions as well in both x and y direction. 

Regards,
Nemesa

> Regards,
> Suraj Kandpal
> 
> >
> > Regards,
> > Nemesa
> >
> > > Regards,
> > > Suraj Kandpal
> > > > +
> > > >  		hsub = 1;
> > > >  		vsub = 1;
> > > >  	} else {
> > > > --
> > > > 2.25.1

Kandpal, Suraj Oct. 16, 2024, 4:25 a.m. UTC | #8

> -----Original Message-----
> From: Garg, Nemesa <[e-mail]>
> Sent: Tuesday, October 15, 2024 9:53 PM
> To: Kandpal, Suraj <[e-mail]>; intel-
> [e-mail]
> Subject: RE: [PATCH] drm/i915/display: Workaround for odd panning for
> planar yuv
> 
> 
> 
> > -----Original Message-----
> > From: Kandpal, Suraj <[e-mail]>
> > Sent: Tuesday, October 15, 2024 8:24 PM
> > To: Garg, Nemesa <[e-mail]>;
> > [e-mail]
> > Subject: RE: [PATCH] drm/i915/display: Workaround for odd panning for
> > planar yuv
> >
> >
> >
> > > -----Original Message-----
> > > From: Garg, Nemesa <[e-mail]>
> > > Sent: Tuesday, October 15, 2024 6:52 PM
> > > To: Kandpal, Suraj <[e-mail]>; intel-
> > > [e-mail]
> > > Subject: RE: [PATCH] drm/i915/display: Workaround for odd panning
> > > for planar yuv
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Kandpal, Suraj <[e-mail]>
> > > > Sent: Tuesday, October 15, 2024 2:33 PM
> > > > To: Garg, Nemesa <[e-mail]>;
> > > > [e-mail]
> > > > Cc: Garg, Nemesa <[e-mail]>
> > > > Subject: RE: [PATCH] drm/i915/display: Workaround for odd panning
> > > > for planar yuv
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Intel-gfx <[e-mail]> On
> > > > > Behalf Of Nemesa Garg
> > > > > Sent: Monday, September 30, 2024 4:54 PM
> > > > > To: [e-mail]
> > > > > Cc: Garg, Nemesa <[e-mail]>
> > > > > 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.
> > > > >
> > > > > WA: 16024459452
> > > > >
> > > > > 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]
> > > > >
> > > > > Signed-off-by: Nemesa Garg <[e-mail]>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8 ++++++++
> > > > >  1 file changed, 8 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..e3401a4f7992 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > > > @@ -1029,6 +1029,14 @@ int
> > > > > intel_plane_check_src_coordinates(struct
> > > > > intel_plane_state *plane_state)
> > > > >  		 * This allows NV12 and P0xx formats to have odd
> size
> > > and/or
> > > > odd
> > > > >  		 * source coordinates on DISPLAY_VER(i915) >= 20
> > > > >  		 */
> > > > > +		/*
> > > > > +		 *  Wa_16023981245 for display version 20.
> > > > > +		 *  Do not support odd x-panning for even xsize for
> NV12.
> > > >
> > > > Only mention WA no here the rest anyone can refer to by going and
> > > > checking the HSD
> > > Sure..will do.
> > > >
> > > > > +		 */
> > > > > +		if (DISPLAY_VER(i915) == 20 && fb->format->format
> ==
> > > > > DRM_FORMAT_NV12 &&
> > > > > +		    src_x % 2 != 0 && src_w % 2 == 0)
> > > > > +			return -EINVAL;
> > > >
> > > > Also rather than return -EINVAL here we set hsub =2 and vsub =1
> > > > this should make sure odd panning is disabled When we have even
> > > > size since we only need to disable odd panning according to hsd
> > > > not even panning this return may end stop panning for even sizes all
> together.
> > > >
> > > Even if I do hsub=2 then also it will return -EINVAL as src_x will
> > > be odd and src_x % hsub will be 1 which will return -EINVAL.
> > > Even panning doesn't get disable that's why I have added the check
> > > src_x %
> > > 2 != 0 to make sure that position in not even and src_w % 2 to check
> > > for even size.
> >
> >
> > That will be fine let the code later that already exists take care of
> > that since we don't Want to abruptly send an -EINVAL in a code block
> > where we were assigning variables.
> > When it fails later as you said it will with a clean debug message too.
> > Secondly when you do return -EINVAL for src_x % 2 != 0 it ends up
> > stopping panning in y direction If the current src_x is odd but
> > panning is being done only in y direction it will return -EINVAL Hence
> > assigning hsub and vsub here make a lot more sense.
> > We can remove the src_x check and keep src_w and add src_h check
> >
> But if I remove  src_x % 2 != 0 then above condition will get true for even
> positions as well in both x and y direction.
> 

Ohkay got it then you can keep the src_x check but still need to add the src_h check along with
And set hsub using if else condition and let it fail later on with a debug message

Regards,
Suraj Kandpal

> Regards,
> Nemesa
> 
> > Regards,
> > Suraj Kandpal
> >
> > >
> > > Regards,
> > > Nemesa
> > >
> > > > Regards,
> > > > Suraj Kandpal
> > > > > +
> > > > >  		hsub = 1;
> > > > >  		vsub = 1;
> > > > >  	} else {
> > > > > --
> > > > > 2.25.1

Nemesa Garg Oct. 16, 2024, 6:26 a.m. UTC | #9

> -----Original Message-----
> From: Kandpal, Suraj <[e-mail]>
> Sent: Wednesday, October 16, 2024 9:55 AM
> To: Garg, Nemesa <[e-mail]>; [e-mail]
> Subject: RE: [PATCH] drm/i915/display: Workaround for odd panning for planar
> yuv
> 
> 
> 
> > -----Original Message-----
> > From: Garg, Nemesa <[e-mail]>
> > Sent: Tuesday, October 15, 2024 9:53 PM
> > To: Kandpal, Suraj <[e-mail]>; intel-
> > [e-mail]
> > Subject: RE: [PATCH] drm/i915/display: Workaround for odd panning for
> > planar yuv
> >
> >
> >
> > > -----Original Message-----
> > > From: Kandpal, Suraj <[e-mail]>
> > > Sent: Tuesday, October 15, 2024 8:24 PM
> > > To: Garg, Nemesa <[e-mail]>;
> > > [e-mail]
> > > Subject: RE: [PATCH] drm/i915/display: Workaround for odd panning
> > > for planar yuv
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Garg, Nemesa <[e-mail]>
> > > > Sent: Tuesday, October 15, 2024 6:52 PM
> > > > To: Kandpal, Suraj <[e-mail]>; intel-
> > > > [e-mail]
> > > > Subject: RE: [PATCH] drm/i915/display: Workaround for odd panning
> > > > for planar yuv
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Kandpal, Suraj <[e-mail]>
> > > > > Sent: Tuesday, October 15, 2024 2:33 PM
> > > > > To: Garg, Nemesa <[e-mail]>;
> > > > > [e-mail]
> > > > > Cc: Garg, Nemesa <[e-mail]>
> > > > > Subject: RE: [PATCH] drm/i915/display: Workaround for odd
> > > > > panning for planar yuv
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Intel-gfx <[e-mail]> On
> > > > > > Behalf Of Nemesa Garg
> > > > > > Sent: Monday, September 30, 2024 4:54 PM
> > > > > > To: [e-mail]
> > > > > > Cc: Garg, Nemesa <[e-mail]>
> > > > > > 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.
> > > > > >
> > > > > > WA: 16024459452
> > > > > >
> > > > > > 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]
> > > > > >
> > > > > > Signed-off-by: Nemesa Garg <[e-mail]>
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8
> > > > > > ++++++++
> > > > > >  1 file changed, 8 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..e3401a4f7992 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > > > > @@ -1029,6 +1029,14 @@ int
> > > > > > intel_plane_check_src_coordinates(struct
> > > > > > intel_plane_state *plane_state)
> > > > > >  		 * This allows NV12 and P0xx formats to have odd
> > size
> > > > and/or
> > > > > odd
> > > > > >  		 * source coordinates on DISPLAY_VER(i915) >= 20
> > > > > >  		 */
> > > > > > +		/*
> > > > > > +		 *  Wa_16023981245 for display version 20.
> > > > > > +		 *  Do not support odd x-panning for even xsize for
> > NV12.
> > > > >
> > > > > Only mention WA no here the rest anyone can refer to by going
> > > > > and checking the HSD
> > > > Sure..will do.
> > > > >
> > > > > > +		 */
> > > > > > +		if (DISPLAY_VER(i915) == 20 && fb->format->format
> > ==
> > > > > > DRM_FORMAT_NV12 &&
> > > > > > +		    src_x % 2 != 0 && src_w % 2 == 0)
> > > > > > +			return -EINVAL;
> > > > >
> > > > > Also rather than return -EINVAL here we set hsub =2 and vsub =1
> > > > > this should make sure odd panning is disabled When we have even
> > > > > size since we only need to disable odd panning according to hsd
> > > > > not even panning this return may end stop panning for even sizes
> > > > > all
> > together.
> > > > >
> > > > Even if I do hsub=2 then also it will return -EINVAL as src_x will
> > > > be odd and src_x % hsub will be 1 which will return -EINVAL.
> > > > Even panning doesn't get disable that's why I have added the check
> > > > src_x %
> > > > 2 != 0 to make sure that position in not even and src_w % 2 to
> > > > check for even size.
> > >
> > >
> > > That will be fine let the code later that already exists take care
> > > of that since we don't Want to abruptly send an -EINVAL in a code
> > > block where we were assigning variables.
> > > When it fails later as you said it will with a clean debug message too.
> > > Secondly when you do return -EINVAL for src_x % 2 != 0 it ends up
> > > stopping panning in y direction If the current src_x is odd but
> > > panning is being done only in y direction it will return -EINVAL
> > > Hence assigning hsub and vsub here make a lot more sense.
> > > We can remove the src_x check and keep src_w and add src_h check
> > >
> > But if I remove  src_x % 2 != 0 then above condition will get true for
> > even positions as well in both x and y direction.
> >
> 
> Ohkay got it then you can keep the src_x check but still need to add the src_h
> check along with And set hsub using if else condition and let it fail later on with a
> debug message
> 
As mentioned in wa I will add a check for src_x only and will add hsub in if-else condition.

Regards,
Nemesa

> Regards,
> Suraj Kandpal
> 
> > Regards,
> > Nemesa
> >
> > > Regards,
> > > Suraj Kandpal
> > >
> > > >
> > > > Regards,
> > > > Nemesa
> > > >
> > > > > Regards,
> > > > > Suraj Kandpal
> > > > > > +
> > > > > >  		hsub = 1;
> > > > > >  		vsub = 1;
> > > > > >  	} else {
> > > > > > --
> > > > > > 2.25.1

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index e979786aa5cf..e3401a4f7992 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -1029,6 +1029,14 @@  int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state)
 		 * This allows NV12 and P0xx formats to have odd size and/or odd
 		 * source coordinates on DISPLAY_VER(i915) >= 20
 		 */
+		/*
+		 *  Wa_16023981245 for display version 20.
+		 *  Do not support odd x-panning for even xsize for NV12.
+		 */
+		if (DISPLAY_VER(i915) == 20 && fb->format->format == DRM_FORMAT_NV12 &&
+		    src_x % 2 != 0 && src_w % 2 == 0)
+			return -EINVAL;
+
 		hsub = 1;
 		vsub = 1;
 	} else {