diff mbox series

[v7,06/11] drm/i915/psr: Modify intel_dp_get_su_granularity to support panel replay

Message ID 20240419121141.2665945-7-jouni.hogander@intel.com (mailing list archive)
State New, archived
Headers show
Series Panel replay selective update support | expand

Commit Message

Hogander, Jouni April 19, 2024, 12:11 p.m. UTC 
Currently intel_dp_get_su_granularity doesn't support panel replay.
This fix modifies it to support panel replay as well.

v2: rely on PSR definitions on common bits

Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
Reviewed-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 62 +++++++++++++++++++++---
 1 file changed, 55 insertions(+), 7 deletions(-)

Comments

Manna, Animesh April 29, 2024, 11:02 a.m. UTC | #1 
> -----Original Message-----
> From: Hogander, Jouni <jouni.hogander@intel.com>
> Sent: Friday, April 19, 2024 5:42 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Manna, Animesh <animesh.manna@intel.com>; Hogander, Jouni
> <jouni.hogander@intel.com>
> Subject: [PATCH v7 06/11] drm/i915/psr: Modify intel_dp_get_su_granularity
> to support panel replay
> 
> Currently intel_dp_get_su_granularity doesn't support panel replay.
> This fix modifies it to support panel replay as well.
> 
> v2: rely on PSR definitions on common bits
> 
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> Reviewed-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 62 +++++++++++++++++++++---
>  1 file changed, 55 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index b94f8e33ed1f..29400fac13c2 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -466,6 +466,40 @@ static u8 intel_dp_get_sink_sync_latency(struct
> intel_dp *intel_dp)
>  	return val;
>  }
> 
> +static u8 intel_dp_get_su_capability(struct intel_dp *intel_dp) {
> +	u8 su_capability;
> +
> +	if (intel_dp->psr.sink_panel_replay_su_support)

While relooking found that good to add a check for DP_PANEL_PANEL_REPLAY_SU_GRANULARITY_REQUIRED (6th bit of dpcd 0xb1). What if it is zero means granularity not needed but will continue to use x-granularity and y-granularity.

Regards,
Animesh

> +		drm_dp_dpcd_read(&intel_dp->aux,
> +				 DP_PANEL_PANEL_REPLAY_X_GRANULARITY,
> +				 &su_capability, 1);
> +	else
> +		su_capability = intel_dp->psr_dpcd[1];
> +
> +	return su_capability;
> +}
> +
> +static unsigned int
> +intel_dp_get_su_x_granularity_offset(struct intel_dp *intel_dp) {
> +	return intel_dp->psr.sink_panel_replay_su_support ?
> +		DP_PANEL_PANEL_REPLAY_X_GRANULARITY :
> +		DP_PSR2_SU_X_GRANULARITY;
> +}
> +
> +static unsigned int
> +intel_dp_get_su_y_granularity_offset(struct intel_dp *intel_dp) {
> +	return intel_dp->psr.sink_panel_replay_su_support ?
> +		DP_PANEL_PANEL_REPLAY_Y_GRANULARITY :
> +		DP_PSR2_SU_Y_GRANULARITY;
> +}
> +
> +/*
> + * Note: Bits related to granularity are same in panel replay and psr
> + * registers. Rely on PSR definitions on these "common" bits.
> + */
>  static void intel_dp_get_su_granularity(struct intel_dp *intel_dp)  {
>  	struct drm_i915_private *i915 = dp_to_i915(intel_dp); @@ -473,18
> +507,29 @@ static void intel_dp_get_su_granularity(struct intel_dp
> *intel_dp)
>  	u16 w;
>  	u8 y;
> 
> -	/* If sink don't have specific granularity requirements set legacy ones
> */
> -	if (!(intel_dp->psr_dpcd[1] &
> DP_PSR2_SU_GRANULARITY_REQUIRED)) {
> +	/*
> +	 * TODO: Do we need to take into account panel supporting both PSR
> and
> +	 * Panel replay?
> +	 */
> +
> +	/*
> +	 * If sink don't have specific granularity requirements set legacy
> +	 * ones.
> +	 */
> +	if (!(intel_dp_get_su_capability(intel_dp) &
> +	      DP_PSR2_SU_GRANULARITY_REQUIRED)) {
>  		/* As PSR2 HW sends full lines, we do not care about x
> granularity */
>  		w = 4;
>  		y = 4;
>  		goto exit;
>  	}
> 
> -	r = drm_dp_dpcd_read(&intel_dp->aux,
> DP_PSR2_SU_X_GRANULARITY, &w, 2);
> +	r = drm_dp_dpcd_read(&intel_dp->aux,
> +			     intel_dp_get_su_x_granularity_offset(intel_dp),
> +			     &w, 2);
>  	if (r != 2)
>  		drm_dbg_kms(&i915->drm,
> -			    "Unable to read
> DP_PSR2_SU_X_GRANULARITY\n");
> +			    "Unable to read selective update x granularity\n");
>  	/*
>  	 * Spec says that if the value read is 0 the default granularity should
>  	 * be used instead.
> @@ -492,10 +537,12 @@ static void intel_dp_get_su_granularity(struct
> intel_dp *intel_dp)
>  	if (r != 2 || w == 0)
>  		w = 4;
> 
> -	r = drm_dp_dpcd_read(&intel_dp->aux,
> DP_PSR2_SU_Y_GRANULARITY, &y, 1);
> +	r = drm_dp_dpcd_read(&intel_dp->aux,
> +			     intel_dp_get_su_y_granularity_offset(intel_dp),
> +			     &y, 1);
>  	if (r != 1) {
>  		drm_dbg_kms(&i915->drm,
> -			    "Unable to read
> DP_PSR2_SU_Y_GRANULARITY\n");
> +			    "Unable to read selective update y granularity\n");
>  		y = 4;
>  	}
>  	if (y == 0)
> @@ -588,7 +635,8 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
>  	if (intel_dp->psr_dpcd[0])
>  		_psr_init_dpcd(intel_dp);
> 
> -	if (intel_dp->psr.sink_psr2_support)
> +	if (intel_dp->psr.sink_psr2_support ||
> +	    intel_dp->psr.sink_panel_replay_su_support)
>  		intel_dp_get_su_granularity(intel_dp);
>  }
> 
> --
> 2.34.1
Hogander, Jouni April 29, 2024, 11:16 a.m. UTC | #2 
On Mon, 2024-04-29 at 11:02 +0000, Manna, Animesh wrote:
> 
> 
> > -----Original Message-----
> > From: Hogander, Jouni <jouni.hogander@intel.com>
> > Sent: Friday, April 19, 2024 5:42 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Manna, Animesh <animesh.manna@intel.com>; Hogander, Jouni
> > <jouni.hogander@intel.com>
> > Subject: [PATCH v7 06/11] drm/i915/psr: Modify
> > intel_dp_get_su_granularity
> > to support panel replay
> > 
> > Currently intel_dp_get_su_granularity doesn't support panel replay.
> > This fix modifies it to support panel replay as well.
> > 
> > v2: rely on PSR definitions on common bits
> > 
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > Reviewed-by: Animesh Manna <animesh.manna@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 62
> > +++++++++++++++++++++---
> >  1 file changed, 55 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index b94f8e33ed1f..29400fac13c2 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -466,6 +466,40 @@ static u8
> > intel_dp_get_sink_sync_latency(struct
> > intel_dp *intel_dp)
> >         return val;
> >  }
> > 
> > +static u8 intel_dp_get_su_capability(struct intel_dp *intel_dp) {
> > +       u8 su_capability;
> > +
> > +       if (intel_dp->psr.sink_panel_replay_su_support)
> 
> While relooking found that good to add a check for
> DP_PANEL_PANEL_REPLAY_SU_GRANULARITY_REQUIRED (6th bit of dpcd 0xb1).
> What if it is zero means granularity not needed but will continue to
> use x-granularity and y-granularity.

Please note it is 5th bit in 0xb1. See my further comment below...

> Regards,
> Animesh
> 
> > +               drm_dp_dpcd_read(&intel_dp->aux,
> > +                               
> > DP_PANEL_PANEL_REPLAY_X_GRANULARITY,
> > +                                &su_capability, 1);
> > +       else
> > +               su_capability = intel_dp->psr_dpcd[1];
> > +
> > +       return su_capability;
> > +}
> > +
> > +static unsigned int
> > +intel_dp_get_su_x_granularity_offset(struct intel_dp *intel_dp) {
> > +       return intel_dp->psr.sink_panel_replay_su_support ?
> > +               DP_PANEL_PANEL_REPLAY_X_GRANULARITY :
> > +               DP_PSR2_SU_X_GRANULARITY;
> > +}
> > +
> > +static unsigned int
> > +intel_dp_get_su_y_granularity_offset(struct intel_dp *intel_dp) {
> > +       return intel_dp->psr.sink_panel_replay_su_support ?
> > +               DP_PANEL_PANEL_REPLAY_Y_GRANULARITY :
> > +               DP_PSR2_SU_Y_GRANULARITY;
> > +}
> > +
> > +/*
> > + * Note: Bits related to granularity are same in panel replay and
> > psr
> > + * registers. Rely on PSR definitions on these "common" bits.
> > + */

Check this comment. See my further comment below...
 
> >  static void intel_dp_get_su_granularity(struct intel_dp
> > *intel_dp)  {
> >         struct drm_i915_private *i915 = dp_to_i915(intel_dp); @@ -
> > 473,18
> > +507,29 @@ static void intel_dp_get_su_granularity(struct intel_dp
> > *intel_dp)
> >         u16 w;
> >         u8 y;
> > 
> > -       /* If sink don't have specific granularity requirements set
> > legacy ones
> > */
> > -       if (!(intel_dp->psr_dpcd[1] &
> > DP_PSR2_SU_GRANULARITY_REQUIRED)) {
> > +       /*
> > +        * TODO: Do we need to take into account panel supporting
> > both PSR
> > and
> > +        * Panel replay?
> > +        */
> > +
> > +       /*
> > +        * If sink don't have specific granularity requirements set
> > legacy
> > +        * ones.
> > +        */
> > +       if (!(intel_dp_get_su_capability(intel_dp) &
> > +             DP_PSR2_SU_GRANULARITY_REQUIRED)) {
> >                 /* As PSR2 HW sends full lines, we do not care
> > about x
> > granularity */
> >                 w = 4;
> >                 y = 4;
> >                 goto exit;
> >         }

This block is taking care of checking bit 5 in 0xb1 and using legacy
ones if no requirements.

BR,

Jouni Högander

> > 
> > -       r = drm_dp_dpcd_read(&intel_dp->aux,
> > DP_PSR2_SU_X_GRANULARITY, &w, 2);
> > +       r = drm_dp_dpcd_read(&intel_dp->aux,
> > +                           
> > intel_dp_get_su_x_granularity_offset(intel_dp),
> > +                            &w, 2);
> >         if (r != 2)
> >                 drm_dbg_kms(&i915->drm,
> > -                           "Unable to read
> > DP_PSR2_SU_X_GRANULARITY\n");
> > +                           "Unable to read selective update x
> > granularity\n");
> >         /*
> >          * Spec says that if the value read is 0 the default
> > granularity should
> >          * be used instead.
> > @@ -492,10 +537,12 @@ static void
> > intel_dp_get_su_granularity(struct
> > intel_dp *intel_dp)
> >         if (r != 2 || w == 0)
> >                 w = 4;
> > 
> > -       r = drm_dp_dpcd_read(&intel_dp->aux,
> > DP_PSR2_SU_Y_GRANULARITY, &y, 1);
> > +       r = drm_dp_dpcd_read(&intel_dp->aux,
> > +                           
> > intel_dp_get_su_y_granularity_offset(intel_dp),
> > +                            &y, 1);
> >         if (r != 1) {
> >                 drm_dbg_kms(&i915->drm,
> > -                           "Unable to read
> > DP_PSR2_SU_Y_GRANULARITY\n");
> > +                           "Unable to read selective update y
> > granularity\n");
> >                 y = 4;
> >         }
> >         if (y == 0)
> > @@ -588,7 +635,8 @@ void intel_psr_init_dpcd(struct intel_dp
> > *intel_dp)
> >         if (intel_dp->psr_dpcd[0])
> >                 _psr_init_dpcd(intel_dp);
> > 
> > -       if (intel_dp->psr.sink_psr2_support)
> > +       if (intel_dp->psr.sink_psr2_support ||
> > +           intel_dp->psr.sink_panel_replay_su_support)
> >                 intel_dp_get_su_granularity(intel_dp);
> >  }
> > 
> > --
> > 2.34.1
>
Manna, Animesh April 29, 2024, 11:34 a.m. UTC | #3 
> -----Original Message-----
> From: Hogander, Jouni <jouni.hogander@intel.com>
> Sent: Monday, April 29, 2024 4:46 PM
> To: Manna, Animesh <animesh.manna@intel.com>; intel-
> gfx@lists.freedesktop.org
> Subject: Re: [PATCH v7 06/11] drm/i915/psr: Modify
> intel_dp_get_su_granularity to support panel replay
> 
> On Mon, 2024-04-29 at 11:02 +0000, Manna, Animesh wrote:
> >
> >
> > > -----Original Message-----
> > > From: Hogander, Jouni <jouni.hogander@intel.com>
> > > Sent: Friday, April 19, 2024 5:42 PM
> > > To: intel-gfx@lists.freedesktop.org
> > > Cc: Manna, Animesh <animesh.manna@intel.com>; Hogander, Jouni
> > > <jouni.hogander@intel.com>
> > > Subject: [PATCH v7 06/11] drm/i915/psr: Modify
> > > intel_dp_get_su_granularity
> > > to support panel replay
> > >
> > > Currently intel_dp_get_su_granularity doesn't support panel replay.
> > > This fix modifies it to support panel replay as well.
> > >
> > > v2: rely on PSR definitions on common bits
> > >
> > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > > Reviewed-by: Animesh Manna <animesh.manna@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_psr.c | 62
> > > +++++++++++++++++++++---
> > >  1 file changed, 55 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index b94f8e33ed1f..29400fac13c2 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -466,6 +466,40 @@ static u8
> > > intel_dp_get_sink_sync_latency(struct
> > > intel_dp *intel_dp)
> > >         return val;
> > >  }
> > >
> > > +static u8 intel_dp_get_su_capability(struct intel_dp *intel_dp) {
> > > +       u8 su_capability;
> > > +
> > > +       if (intel_dp->psr.sink_panel_replay_su_support)
> >
> > While relooking found that good to add a check for
> > DP_PANEL_PANEL_REPLAY_SU_GRANULARITY_REQUIRED (6th bit of dpcd
> 0xb1).
> > What if it is zero means granularity not needed but will continue to
> > use x-granularity and y-granularity.
> 
> Please note it is 5th bit in 0xb1. See my further comment below...

Same I was referring,  6th bit index 5 as it starts from 0.

> 
> > Regards,
> > Animesh
> >
> > > +               drm_dp_dpcd_read(&intel_dp->aux,
> > > +
> > > DP_PANEL_PANEL_REPLAY_X_GRANULARITY,
> > > +                                &su_capability, 1);
> > > +       else
> > > +               su_capability = intel_dp->psr_dpcd[1];
> > > +
> > > +       return su_capability;
> > > +}
> > > +
> > > +static unsigned int
> > > +intel_dp_get_su_x_granularity_offset(struct intel_dp *intel_dp) {
> > > +       return intel_dp->psr.sink_panel_replay_su_support ?
> > > +               DP_PANEL_PANEL_REPLAY_X_GRANULARITY :
> > > +               DP_PSR2_SU_X_GRANULARITY;
> > > +}
> > > +
> > > +static unsigned int
> > > +intel_dp_get_su_y_granularity_offset(struct intel_dp *intel_dp) {
> > > +       return intel_dp->psr.sink_panel_replay_su_support ?
> > > +               DP_PANEL_PANEL_REPLAY_Y_GRANULARITY :
> > > +               DP_PSR2_SU_Y_GRANULARITY;
> > > +}
> > > +
> > > +/*
> > > + * Note: Bits related to granularity are same in panel replay and
> > > psr
> > > + * registers. Rely on PSR definitions on these "common" bits.
> > > + */
> 
> Check this comment. See my further comment below...

Agree bit position is same.

> 
> > >  static void intel_dp_get_su_granularity(struct intel_dp
> > > *intel_dp)  {
> > >         struct drm_i915_private *i915 = dp_to_i915(intel_dp); @@ -
> > > 473,18
> > > +507,29 @@ static void intel_dp_get_su_granularity(struct intel_dp
> > > *intel_dp)
> > >         u16 w;
> > >         u8 y;
> > >
> > > -       /* If sink don't have specific granularity requirements set
> > > legacy ones
> > > */
> > > -       if (!(intel_dp->psr_dpcd[1] &
> > > DP_PSR2_SU_GRANULARITY_REQUIRED)) {
> > > +       /*
> > > +        * TODO: Do we need to take into account panel supporting
> > > both PSR
> > > and
> > > +        * Panel replay?
> > > +        */
> > > +
> > > +       /*
> > > +        * If sink don't have specific granularity requirements set
> > > legacy
> > > +        * ones.
> > > +        */
> > > +       if (!(intel_dp_get_su_capability(intel_dp) &
> > > +             DP_PSR2_SU_GRANULARITY_REQUIRED)) {
> > >                 /* As PSR2 HW sends full lines, we do not care
> > > about x
> > > granularity */
> > >                 w = 4;
> > >                 y = 4;
> > >                 goto exit;
> > >         }
> 
> This block is taking care of checking bit 5 in 0xb1 and using legacy
> ones if no requirements.

intel_dp_get_su_capability is reading from DP_PANEL_PANEL_REPLAY_X_GRANULARITY which Is dpcd 0xb2. Am I missing something?

Regards,
Animesh

> 
> BR,
> 
> Jouni Högander
> 
> > >
> > > -       r = drm_dp_dpcd_read(&intel_dp->aux,
> > > DP_PSR2_SU_X_GRANULARITY, &w, 2);
> > > +       r = drm_dp_dpcd_read(&intel_dp->aux,
> > > +
> > > intel_dp_get_su_x_granularity_offset(intel_dp),
> > > +                            &w, 2);
> > >         if (r != 2)
> > >                 drm_dbg_kms(&i915->drm,
> > > -                           "Unable to read
> > > DP_PSR2_SU_X_GRANULARITY\n");
> > > +                           "Unable to read selective update x
> > > granularity\n");
> > >         /*
> > >          * Spec says that if the value read is 0 the default
> > > granularity should
> > >          * be used instead.
> > > @@ -492,10 +537,12 @@ static void
> > > intel_dp_get_su_granularity(struct
> > > intel_dp *intel_dp)
> > >         if (r != 2 || w == 0)
> > >                 w = 4;
> > >
> > > -       r = drm_dp_dpcd_read(&intel_dp->aux,
> > > DP_PSR2_SU_Y_GRANULARITY, &y, 1);
> > > +       r = drm_dp_dpcd_read(&intel_dp->aux,
> > > +
> > > intel_dp_get_su_y_granularity_offset(intel_dp),
> > > +                            &y, 1);
> > >         if (r != 1) {
> > >                 drm_dbg_kms(&i915->drm,
> > > -                           "Unable to read
> > > DP_PSR2_SU_Y_GRANULARITY\n");
> > > +                           "Unable to read selective update y
> > > granularity\n");
> > >                 y = 4;
> > >         }
> > >         if (y == 0)
> > > @@ -588,7 +635,8 @@ void intel_psr_init_dpcd(struct intel_dp
> > > *intel_dp)
> > >         if (intel_dp->psr_dpcd[0])
> > >                 _psr_init_dpcd(intel_dp);
> > >
> > > -       if (intel_dp->psr.sink_psr2_support)
> > > +       if (intel_dp->psr.sink_psr2_support ||
> > > +           intel_dp->psr.sink_panel_replay_su_support)
> > >                 intel_dp_get_su_granularity(intel_dp);
> > >  }
> > >
> > > --
> > > 2.34.1
> >
Manna, Animesh April 29, 2024, 11:45 a.m. UTC | #4 
> -----Original Message-----
> From: Manna, Animesh
> Sent: Monday, April 29, 2024 5:04 PM
> To: Hogander, Jouni <jouni.hogander@intel.com>; intel-
> gfx@lists.freedesktop.org
> Subject: RE: [PATCH v7 06/11] drm/i915/psr: Modify
> intel_dp_get_su_granularity to support panel replay
> 
> 
> 
> > -----Original Message-----
> > From: Hogander, Jouni <jouni.hogander@intel.com>
> > Sent: Monday, April 29, 2024 4:46 PM
> > To: Manna, Animesh <animesh.manna@intel.com>; intel-
> > gfx@lists.freedesktop.org
> > Subject: Re: [PATCH v7 06/11] drm/i915/psr: Modify
> > intel_dp_get_su_granularity to support panel replay
> >
> > On Mon, 2024-04-29 at 11:02 +0000, Manna, Animesh wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Hogander, Jouni <jouni.hogander@intel.com>
> > > > Sent: Friday, April 19, 2024 5:42 PM
> > > > To: intel-gfx@lists.freedesktop.org
> > > > Cc: Manna, Animesh <animesh.manna@intel.com>; Hogander, Jouni
> > > > <jouni.hogander@intel.com>
> > > > Subject: [PATCH v7 06/11] drm/i915/psr: Modify
> > > > intel_dp_get_su_granularity to support panel replay
> > > >
> > > > Currently intel_dp_get_su_granularity doesn't support panel replay.
> > > > This fix modifies it to support panel replay as well.
> > > >
> > > > v2: rely on PSR definitions on common bits
> > > >
> > > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > > > Reviewed-by: Animesh Manna <animesh.manna@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_psr.c | 62
> > > > +++++++++++++++++++++---
> > > >  1 file changed, 55 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > index b94f8e33ed1f..29400fac13c2 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > @@ -466,6 +466,40 @@ static u8
> > > > intel_dp_get_sink_sync_latency(struct
> > > > intel_dp *intel_dp)
> > > >         return val;
> > > >  }
> > > >
> > > > +static u8 intel_dp_get_su_capability(struct intel_dp *intel_dp) {
> > > > +       u8 su_capability;
> > > > +
> > > > +       if (intel_dp->psr.sink_panel_replay_su_support)
> > >
> > > While relooking found that good to add a check for
> > > DP_PANEL_PANEL_REPLAY_SU_GRANULARITY_REQUIRED (6th bit of dpcd
> > 0xb1).
> > > What if it is zero means granularity not needed but will continue to
> > > use x-granularity and y-granularity.
> >
> > Please note it is 5th bit in 0xb1. See my further comment below...
> 
> Same I was referring,  6th bit index 5 as it starts from 0.
> 
> >
> > > Regards,
> > > Animesh
> > >
> > > > +               drm_dp_dpcd_read(&intel_dp->aux,
> > > > +
> > > > DP_PANEL_PANEL_REPLAY_X_GRANULARITY,
> > > > +                                &su_capability, 1);

I think instead of DP_PANEL_PANEL_REPLAY_X_GRANULARITY it should be DP_PANEL_PANEL_REPLAY_CAPABILITY, everything else will be fine.

Regards,
Animesh
> > > > +       else
> > > > +               su_capability = intel_dp->psr_dpcd[1];
> > > > +
> > > > +       return su_capability;
> > > > +}
> > > > +
> > > > +static unsigned int
> > > > +intel_dp_get_su_x_granularity_offset(struct intel_dp *intel_dp) {
> > > > +       return intel_dp->psr.sink_panel_replay_su_support ?
> > > > +               DP_PANEL_PANEL_REPLAY_X_GRANULARITY :
> > > > +               DP_PSR2_SU_X_GRANULARITY; }
> > > > +
> > > > +static unsigned int
> > > > +intel_dp_get_su_y_granularity_offset(struct intel_dp *intel_dp) {
> > > > +       return intel_dp->psr.sink_panel_replay_su_support ?
> > > > +               DP_PANEL_PANEL_REPLAY_Y_GRANULARITY :
> > > > +               DP_PSR2_SU_Y_GRANULARITY; }
> > > > +
> > > > +/*
> > > > + * Note: Bits related to granularity are same in panel replay and
> > > > psr
> > > > + * registers. Rely on PSR definitions on these "common" bits.
> > > > + */
> >
> > Check this comment. See my further comment below...
> 
> Agree bit position is same.
> 
> >
> > > >  static void intel_dp_get_su_granularity(struct intel_dp
> > > > *intel_dp)  {
> > > >         struct drm_i915_private *i915 = dp_to_i915(intel_dp); @@ -
> > > > 473,18
> > > > +507,29 @@ static void intel_dp_get_su_granularity(struct intel_dp
> > > > *intel_dp)
> > > >         u16 w;
> > > >         u8 y;
> > > >
> > > > -       /* If sink don't have specific granularity requirements
> > > > set legacy ones */
> > > > -       if (!(intel_dp->psr_dpcd[1] &
> > > > DP_PSR2_SU_GRANULARITY_REQUIRED)) {
> > > > +       /*
> > > > +        * TODO: Do we need to take into account panel supporting
> > > > both PSR
> > > > and
> > > > +        * Panel replay?
> > > > +        */
> > > > +
> > > > +       /*
> > > > +        * If sink don't have specific granularity requirements
> > > > +set
> > > > legacy
> > > > +        * ones.
> > > > +        */
> > > > +       if (!(intel_dp_get_su_capability(intel_dp) &
> > > > +             DP_PSR2_SU_GRANULARITY_REQUIRED)) {
> > > >                 /* As PSR2 HW sends full lines, we do not care
> > > > about x granularity */
> > > >                 w = 4;
> > > >                 y = 4;
> > > >                 goto exit;
> > > >         }
> >
> > This block is taking care of checking bit 5 in 0xb1 and using legacy
> > ones if no requirements.
> 
> intel_dp_get_su_capability is reading from
> DP_PANEL_PANEL_REPLAY_X_GRANULARITY which Is dpcd 0xb2. Am I
> missing something?
> 
> Regards,
> Animesh
> 
> >
> > BR,
> >
> > Jouni Högander
> >
> > > >
> > > > -       r = drm_dp_dpcd_read(&intel_dp->aux,
> > > > DP_PSR2_SU_X_GRANULARITY, &w, 2);
> > > > +       r = drm_dp_dpcd_read(&intel_dp->aux,
> > > > +
> > > > intel_dp_get_su_x_granularity_offset(intel_dp),
> > > > +                            &w, 2);
> > > >         if (r != 2)
> > > >                 drm_dbg_kms(&i915->drm,
> > > > -                           "Unable to read
> > > > DP_PSR2_SU_X_GRANULARITY\n");
> > > > +                           "Unable to read selective update x
> > > > granularity\n");
> > > >         /*
> > > >          * Spec says that if the value read is 0 the default
> > > > granularity should
> > > >          * be used instead.
> > > > @@ -492,10 +537,12 @@ static void
> > > > intel_dp_get_su_granularity(struct
> > > > intel_dp *intel_dp)
> > > >         if (r != 2 || w == 0)
> > > >                 w = 4;
> > > >
> > > > -       r = drm_dp_dpcd_read(&intel_dp->aux,
> > > > DP_PSR2_SU_Y_GRANULARITY, &y, 1);
> > > > +       r = drm_dp_dpcd_read(&intel_dp->aux,
> > > > +
> > > > intel_dp_get_su_y_granularity_offset(intel_dp),
> > > > +                            &y, 1);
> > > >         if (r != 1) {
> > > >                 drm_dbg_kms(&i915->drm,
> > > > -                           "Unable to read
> > > > DP_PSR2_SU_Y_GRANULARITY\n");
> > > > +                           "Unable to read selective update y
> > > > granularity\n");
> > > >                 y = 4;
> > > >         }
> > > >         if (y == 0)
> > > > @@ -588,7 +635,8 @@ void intel_psr_init_dpcd(struct intel_dp
> > > > *intel_dp)
> > > >         if (intel_dp->psr_dpcd[0])
> > > >                 _psr_init_dpcd(intel_dp);
> > > >
> > > > -       if (intel_dp->psr.sink_psr2_support)
> > > > +       if (intel_dp->psr.sink_psr2_support ||
> > > > +           intel_dp->psr.sink_panel_replay_su_support)
> > > >                 intel_dp_get_su_granularity(intel_dp);
> > > >  }
> > > >
> > > > --
> > > > 2.34.1
> > >
Hogander, Jouni April 29, 2024, 11:46 a.m. UTC | #5 
On Mon, 2024-04-29 at 11:45 +0000, Manna, Animesh wrote:
> 
> 
> > -----Original Message-----
> > From: Manna, Animesh
> > Sent: Monday, April 29, 2024 5:04 PM
> > To: Hogander, Jouni <jouni.hogander@intel.com>; intel-
> > gfx@lists.freedesktop.org
> > Subject: RE: [PATCH v7 06/11] drm/i915/psr: Modify
> > intel_dp_get_su_granularity to support panel replay
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: Hogander, Jouni <jouni.hogander@intel.com>
> > > Sent: Monday, April 29, 2024 4:46 PM
> > > To: Manna, Animesh <animesh.manna@intel.com>; intel-
> > > gfx@lists.freedesktop.org
> > > Subject: Re: [PATCH v7 06/11] drm/i915/psr: Modify
> > > intel_dp_get_su_granularity to support panel replay
> > > 
> > > On Mon, 2024-04-29 at 11:02 +0000, Manna, Animesh wrote:
> > > > 
> > > > 
> > > > > -----Original Message-----
> > > > > From: Hogander, Jouni <jouni.hogander@intel.com>
> > > > > Sent: Friday, April 19, 2024 5:42 PM
> > > > > To: intel-gfx@lists.freedesktop.org
> > > > > Cc: Manna, Animesh <animesh.manna@intel.com>; Hogander, Jouni
> > > > > <jouni.hogander@intel.com>
> > > > > Subject: [PATCH v7 06/11] drm/i915/psr: Modify
> > > > > intel_dp_get_su_granularity to support panel replay
> > > > > 
> > > > > Currently intel_dp_get_su_granularity doesn't support panel
> > > > > replay.
> > > > > This fix modifies it to support panel replay as well.
> > > > > 
> > > > > v2: rely on PSR definitions on common bits
> > > > > 
> > > > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > > > > Reviewed-by: Animesh Manna <animesh.manna@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/display/intel_psr.c | 62
> > > > > +++++++++++++++++++++---
> > > > >  1 file changed, 55 insertions(+), 7 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > index b94f8e33ed1f..29400fac13c2 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > @@ -466,6 +466,40 @@ static u8
> > > > > intel_dp_get_sink_sync_latency(struct
> > > > > intel_dp *intel_dp)
> > > > >         return val;
> > > > >  }
> > > > > 
> > > > > +static u8 intel_dp_get_su_capability(struct intel_dp
> > > > > *intel_dp) {
> > > > > +       u8 su_capability;
> > > > > +
> > > > > +       if (intel_dp->psr.sink_panel_replay_su_support)
> > > > 
> > > > While relooking found that good to add a check for
> > > > DP_PANEL_PANEL_REPLAY_SU_GRANULARITY_REQUIRED (6th bit of dpcd
> > > 0xb1).
> > > > What if it is zero means granularity not needed but will
> > > > continue to
> > > > use x-granularity and y-granularity.
> > > 
> > > Please note it is 5th bit in 0xb1. See my further comment
> > > below...
> > 
> > Same I was referring,  6th bit index 5 as it starts from 0.
> > 
> > > 
> > > > Regards,
> > > > Animesh
> > > > 
> > > > > +               drm_dp_dpcd_read(&intel_dp->aux,
> > > > > +
> > > > > DP_PANEL_PANEL_REPLAY_X_GRANULARITY,
> > > > > +                                &su_capability, 1);
> 
> I think instead of DP_PANEL_PANEL_REPLAY_X_GRANULARITY it should be
> DP_PANEL_PANEL_REPLAY_CAPABILITY, everything else will be fine.

Yes. Just noticed that as well. Good catch here. I will update my set.

BR,

Jouni Högander

> 
> Regards,
> Animesh
> > > > > +       else
> > > > > +               su_capability = intel_dp->psr_dpcd[1];
> > > > > +
> > > > > +       return su_capability;
> > > > > +}
> > > > > +
> > > > > +static unsigned int
> > > > > +intel_dp_get_su_x_granularity_offset(struct intel_dp
> > > > > *intel_dp) {
> > > > > +       return intel_dp->psr.sink_panel_replay_su_support ?
> > > > > +               DP_PANEL_PANEL_REPLAY_X_GRANULARITY :
> > > > > +               DP_PSR2_SU_X_GRANULARITY; }
> > > > > +
> > > > > +static unsigned int
> > > > > +intel_dp_get_su_y_granularity_offset(struct intel_dp
> > > > > *intel_dp) {
> > > > > +       return intel_dp->psr.sink_panel_replay_su_support ?
> > > > > +               DP_PANEL_PANEL_REPLAY_Y_GRANULARITY :
> > > > > +               DP_PSR2_SU_Y_GRANULARITY; }
> > > > > +
> > > > > +/*
> > > > > + * Note: Bits related to granularity are same in panel
> > > > > replay and
> > > > > psr
> > > > > + * registers. Rely on PSR definitions on these "common"
> > > > > bits.
> > > > > + */
> > > 
> > > Check this comment. See my further comment below...
> > 
> > Agree bit position is same.
> > 
> > > 
> > > > >  static void intel_dp_get_su_granularity(struct intel_dp
> > > > > *intel_dp)  {
> > > > >         struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > > > > @@ -
> > > > > 473,18
> > > > > +507,29 @@ static void intel_dp_get_su_granularity(struct
> > > > > intel_dp
> > > > > *intel_dp)
> > > > >         u16 w;
> > > > >         u8 y;
> > > > > 
> > > > > -       /* If sink don't have specific granularity
> > > > > requirements
> > > > > set legacy ones */
> > > > > -       if (!(intel_dp->psr_dpcd[1] &
> > > > > DP_PSR2_SU_GRANULARITY_REQUIRED)) {
> > > > > +       /*
> > > > > +        * TODO: Do we need to take into account panel
> > > > > supporting
> > > > > both PSR
> > > > > and
> > > > > +        * Panel replay?
> > > > > +        */
> > > > > +
> > > > > +       /*
> > > > > +        * If sink don't have specific granularity
> > > > > requirements
> > > > > +set
> > > > > legacy
> > > > > +        * ones.
> > > > > +        */
> > > > > +       if (!(intel_dp_get_su_capability(intel_dp) &
> > > > > +             DP_PSR2_SU_GRANULARITY_REQUIRED)) {
> > > > >                 /* As PSR2 HW sends full lines, we do not
> > > > > care
> > > > > about x granularity */
> > > > >                 w = 4;
> > > > >                 y = 4;
> > > > >                 goto exit;
> > > > >         }
> > > 
> > > This block is taking care of checking bit 5 in 0xb1 and using
> > > legacy
> > > ones if no requirements.
> > 
> > intel_dp_get_su_capability is reading from
> > DP_PANEL_PANEL_REPLAY_X_GRANULARITY which Is dpcd 0xb2. Am I
> > missing something?
> > 
> > Regards,
> > Animesh
> > 
> > > 
> > > BR,
> > > 
> > > Jouni Högander
> > > 
> > > > > 
> > > > > -       r = drm_dp_dpcd_read(&intel_dp->aux,
> > > > > DP_PSR2_SU_X_GRANULARITY, &w, 2);
> > > > > +       r = drm_dp_dpcd_read(&intel_dp->aux,
> > > > > +
> > > > > intel_dp_get_su_x_granularity_offset(intel_dp),
> > > > > +                            &w, 2);
> > > > >         if (r != 2)
> > > > >                 drm_dbg_kms(&i915->drm,
> > > > > -                           "Unable to read
> > > > > DP_PSR2_SU_X_GRANULARITY\n");
> > > > > +                           "Unable to read selective update
> > > > > x
> > > > > granularity\n");
> > > > >         /*
> > > > >          * Spec says that if the value read is 0 the default
> > > > > granularity should
> > > > >          * be used instead.
> > > > > @@ -492,10 +537,12 @@ static void
> > > > > intel_dp_get_su_granularity(struct
> > > > > intel_dp *intel_dp)
> > > > >         if (r != 2 || w == 0)
> > > > >                 w = 4;
> > > > > 
> > > > > -       r = drm_dp_dpcd_read(&intel_dp->aux,
> > > > > DP_PSR2_SU_Y_GRANULARITY, &y, 1);
> > > > > +       r = drm_dp_dpcd_read(&intel_dp->aux,
> > > > > +
> > > > > intel_dp_get_su_y_granularity_offset(intel_dp),
> > > > > +                            &y, 1);
> > > > >         if (r != 1) {
> > > > >                 drm_dbg_kms(&i915->drm,
> > > > > -                           "Unable to read
> > > > > DP_PSR2_SU_Y_GRANULARITY\n");
> > > > > +                           "Unable to read selective update
> > > > > y
> > > > > granularity\n");
> > > > >                 y = 4;
> > > > >         }
> > > > >         if (y == 0)
> > > > > @@ -588,7 +635,8 @@ void intel_psr_init_dpcd(struct intel_dp
> > > > > *intel_dp)
> > > > >         if (intel_dp->psr_dpcd[0])
> > > > >                 _psr_init_dpcd(intel_dp);
> > > > > 
> > > > > -       if (intel_dp->psr.sink_psr2_support)
> > > > > +       if (intel_dp->psr.sink_psr2_support ||
> > > > > +           intel_dp->psr.sink_panel_replay_su_support)
> > > > >                 intel_dp_get_su_granularity(intel_dp);
> > > > >  }
> > > > > 
> > > > > --
> > > > > 2.34.1
> > > > 
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index b94f8e33ed1f..29400fac13c2 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -466,6 +466,40 @@  static u8 intel_dp_get_sink_sync_latency(struct intel_dp *intel_dp)
 	return val;
 }
 
+static u8 intel_dp_get_su_capability(struct intel_dp *intel_dp)
+{
+	u8 su_capability;
+
+	if (intel_dp->psr.sink_panel_replay_su_support)
+		drm_dp_dpcd_read(&intel_dp->aux,
+				 DP_PANEL_PANEL_REPLAY_X_GRANULARITY,
+				 &su_capability, 1);
+	else
+		su_capability = intel_dp->psr_dpcd[1];
+
+	return su_capability;
+}
+
+static unsigned int
+intel_dp_get_su_x_granularity_offset(struct intel_dp *intel_dp)
+{
+	return intel_dp->psr.sink_panel_replay_su_support ?
+		DP_PANEL_PANEL_REPLAY_X_GRANULARITY :
+		DP_PSR2_SU_X_GRANULARITY;
+}
+
+static unsigned int
+intel_dp_get_su_y_granularity_offset(struct intel_dp *intel_dp)
+{
+	return intel_dp->psr.sink_panel_replay_su_support ?
+		DP_PANEL_PANEL_REPLAY_Y_GRANULARITY :
+		DP_PSR2_SU_Y_GRANULARITY;
+}
+
+/*
+ * Note: Bits related to granularity are same in panel replay and psr
+ * registers. Rely on PSR definitions on these "common" bits.
+ */
 static void intel_dp_get_su_granularity(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
@@ -473,18 +507,29 @@  static void intel_dp_get_su_granularity(struct intel_dp *intel_dp)
 	u16 w;
 	u8 y;
 
-	/* If sink don't have specific granularity requirements set legacy ones */
-	if (!(intel_dp->psr_dpcd[1] & DP_PSR2_SU_GRANULARITY_REQUIRED)) {
+	/*
+	 * TODO: Do we need to take into account panel supporting both PSR and
+	 * Panel replay?
+	 */
+
+	/*
+	 * If sink don't have specific granularity requirements set legacy
+	 * ones.
+	 */
+	if (!(intel_dp_get_su_capability(intel_dp) &
+	      DP_PSR2_SU_GRANULARITY_REQUIRED)) {
 		/* As PSR2 HW sends full lines, we do not care about x granularity */
 		w = 4;
 		y = 4;
 		goto exit;
 	}
 
-	r = drm_dp_dpcd_read(&intel_dp->aux, DP_PSR2_SU_X_GRANULARITY, &w, 2);
+	r = drm_dp_dpcd_read(&intel_dp->aux,
+			     intel_dp_get_su_x_granularity_offset(intel_dp),
+			     &w, 2);
 	if (r != 2)
 		drm_dbg_kms(&i915->drm,
-			    "Unable to read DP_PSR2_SU_X_GRANULARITY\n");
+			    "Unable to read selective update x granularity\n");
 	/*
 	 * Spec says that if the value read is 0 the default granularity should
 	 * be used instead.
@@ -492,10 +537,12 @@  static void intel_dp_get_su_granularity(struct intel_dp *intel_dp)
 	if (r != 2 || w == 0)
 		w = 4;
 
-	r = drm_dp_dpcd_read(&intel_dp->aux, DP_PSR2_SU_Y_GRANULARITY, &y, 1);
+	r = drm_dp_dpcd_read(&intel_dp->aux,
+			     intel_dp_get_su_y_granularity_offset(intel_dp),
+			     &y, 1);
 	if (r != 1) {
 		drm_dbg_kms(&i915->drm,
-			    "Unable to read DP_PSR2_SU_Y_GRANULARITY\n");
+			    "Unable to read selective update y granularity\n");
 		y = 4;
 	}
 	if (y == 0)
@@ -588,7 +635,8 @@  void intel_psr_init_dpcd(struct intel_dp *intel_dp)
 	if (intel_dp->psr_dpcd[0])
 		_psr_init_dpcd(intel_dp);
 
-	if (intel_dp->psr.sink_psr2_support)
+	if (intel_dp->psr.sink_psr2_support ||
+	    intel_dp->psr.sink_panel_replay_su_support)
 		intel_dp_get_su_granularity(intel_dp);
 }