From: "Hogander, Jouni" <jouni.hogander@intel.com>
To: "Manna, Animesh" <animesh.manna@intel.com>,
"intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH v7 06/11] drm/i915/psr: Modify intel_dp_get_su_granularity to support panel replay
Date: Mon, 29 Apr 2024 11:46:49 +0000 [thread overview]
Message-ID: <8bb9cd2612e3df23aed9c9a3e2978cc73f859c86.camel@intel.com> (raw)
In-Reply-To: <PH7PR11MB59810DA6F96CA4FA0127A3B7F91B2@PH7PR11MB5981.namprd11.prod.outlook.com>
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
> > > >
>
next prev parent reply other threads:[~2024-04-29 11:46 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-19 12:11 [PATCH v7 00/11] Panel replay selective update support Jouni Högander
2024-04-19 12:11 ` [PATCH v7 01/11] drm/i915/psr: Rename has_psr2 as has_sel_update Jouni Högander
2024-04-19 12:11 ` [PATCH v7 02/11] drm/i915/dp: Use always vsc revision 0x6 for Panel Replay Jouni Högander
2024-04-19 12:11 ` [PATCH v7 03/11] drm/i915/psr: Rename psr2_enabled as sel_update_enabled Jouni Högander
2024-04-19 12:11 ` [PATCH v7 04/11] drm/panelreplay: dpcd register definition for panelreplay SU Jouni Högander
2024-04-19 12:11 ` [PATCH v7 05/11] drm/i915/psr: Detect panel replay selective update support Jouni Högander
2024-04-19 12:11 ` [PATCH v7 06/11] drm/i915/psr: Modify intel_dp_get_su_granularity to support panel replay Jouni Högander
2024-04-29 11:02 ` Manna, Animesh
2024-04-29 11:16 ` Hogander, Jouni
2024-04-29 11:34 ` Manna, Animesh
2024-04-29 11:45 ` Manna, Animesh
2024-04-29 11:46 ` Hogander, Jouni [this message]
2024-04-19 12:11 ` [PATCH v7 07/11] drm/i915/psr: Panel replay uses SRD_STATUS to track it's status Jouni Högander
2024-04-19 12:11 ` [PATCH v7 08/11] drm/i915/psr: Do not apply workarounds in case of panel replay Jouni Högander
2024-04-19 12:11 ` [PATCH v7 09/11] drm/i915/psr: Update PSR module parameter descriptions Jouni Högander
2024-04-19 12:11 ` [PATCH v7 10/11] drm/i915/psr: Split intel_psr2_config_valid for panel replay Jouni Högander
2024-04-19 12:11 ` [PATCH v7 11/11] drm/i915/psr: Add panel replay sel update support to debugfs interface Jouni Högander
2024-04-19 12:36 ` ✗ Fi.CI.CHECKPATCH: warning for Panel replay selective update support (rev7) Patchwork
2024-04-19 12:36 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-04-19 12:43 ` ✗ Fi.CI.BAT: failure " Patchwork
2024-04-22 6:33 ` ✗ Fi.CI.CHECKPATCH: warning for Panel replay selective update support (rev8) Patchwork
2024-04-22 6:33 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-04-22 6:41 ` ✓ Fi.CI.BAT: success " Patchwork
2024-04-22 8:37 ` ✗ Fi.CI.IGT: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8bb9cd2612e3df23aed9c9a3e2978cc73f859c86.camel@intel.com \
--to=jouni.hogander@intel.com \
--cc=animesh.manna@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).