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

Manna, Animesh animesh.manna at intel.com
Mon Apr 29 11:45:15 UTC 2024



> -----Original Message-----
> From: Manna, Animesh
> Sent: Monday, April 29, 2024 5:04 PM
> To: Hogander, Jouni <jouni.hogander at intel.com>; intel-
> gfx at 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 at intel.com>
> > Sent: Monday, April 29, 2024 4:46 PM
> > To: Manna, Animesh <animesh.manna at intel.com>; intel-
> > gfx at 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 at intel.com>
> > > > Sent: Friday, April 19, 2024 5:42 PM
> > > > To: intel-gfx at lists.freedesktop.org
> > > > Cc: Manna, Animesh <animesh.manna at intel.com>; Hogander, Jouni
> > > > <jouni.hogander at 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 at intel.com>
> > > > Reviewed-by: Animesh Manna <animesh.manna at 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
> > >



More information about the Intel-gfx mailing list