| Message ID | 20240509032922.1145558-2-suraj.kandpal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/pps: Disable DPLS_GATING around pps sequence | expand |
On Thu, May 09, 2024 at 08:59:23AM +0530, Suraj Kandpal wrote: > Disable bit 29 of SCLKGATE_DIS register around pps sequence > when we turn panel power on. > > --v2 > -Squash two commit together [Jani] > -Use IS_DISPLAY_VER [Jani] > -Fix multiline comment [Jani] > > --v3 > -Define register in a more appropriate place [Mitul] > > Bspec: 49304 > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_pps.c | 12 ++++++++++++ > drivers/gpu/drm/i915/i915_reg.h | 4 ++++ > 2 files changed, 16 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c > index 0ccbf9a85914..d774aeb1673e 100644 > --- a/drivers/gpu/drm/i915/display/intel_pps.c > +++ b/drivers/gpu/drm/i915/display/intel_pps.c > @@ -948,6 +948,14 @@ void intel_pps_on_unlocked(struct intel_dp *intel_dp) > intel_de_posting_read(dev_priv, pp_ctrl_reg); > } > > + /* > + * WA: 16023567976 > + * Disable DPLS gating around power sequence. > + */ > + if (IS_DISPLAY_VER(dev_priv, 12, 14)) The issue has supposedly existed since at least BXT. It was documented as w/a #1124 there: https://patchwork.freedesktop.org/series/70655/ The original w/a called for keeping the clock gating disabled between the PP_ON_DELAYS and PP_CONTROL writes, which would have been annoying to implement so I went with the extra delay instead. But if the new approach of just toggle the clock gating around the PP_CONTROL write works then that is definitely better. Sadly I wasn't able to reproduce this issue locally. Gave it a decent try on GLK, TGL, and ADL, but no joy. So can't be sure this actually works. I suppose technically it doesn't matter for us since we always use the VDD override anyway, but no harm in having the w/a implemented anyway in case we ever change that. > + intel_de_rmw(dev_priv, SCLKGATE_DIS, > + 0, DPLS_GATING_DISABLE); IIRC on BXT/GLK we need to poke at some north clock gating register. And on BXT/GLK, and ICP+ we can have two power sequencers so we probably want to poke the bits for both of them. The other issue is that we are poking at these register from multiple places, so we probably need a lock to protect it. I'm think we could have just a single chicken_lock or something for these kinds of use cases. > + > pp |= PANEL_POWER_ON; > if (!IS_IRONLAKE(dev_priv)) > pp |= PANEL_POWER_RESET; > @@ -958,6 +966,10 @@ void intel_pps_on_unlocked(struct intel_dp *intel_dp) > wait_panel_on(intel_dp); > intel_dp->pps.last_power_on = jiffies; > > + if (IS_DISPLAY_VER(dev_priv, 12, 14)) > + intel_de_rmw(dev_priv, SCLKGATE_DIS, > + DPLS_GATING_DISABLE, 0); > + > if (IS_IRONLAKE(dev_priv)) { > pp |= PANEL_POWER_RESET; /* restore panel reset bit */ > intel_de_write(dev_priv, pp_ctrl_reg, pp); > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 5670eee4a498..4cc82360298b 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -5151,6 +5151,10 @@ enum skl_power_gate { > > #define _MMIO_PLANE_GAMC(plane, i, a, b) _MMIO(_PIPE(plane, a, b) + (i) * 4) > > +/* SCLKGATE_DIS */ > +#define SCLKGATE_DIS _MMIO(0xc2020) We already have that register. > +#define DPLS_GATING_DISABLE REG_BIT(29) > + > /* Plane CSC Registers */ > #define _PLANE_CSC_RY_GY_1_A 0x70210 > #define _PLANE_CSC_RY_GY_2_A 0x70310 > -- > 2.43.2
> -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Suraj > Kandpal > Sent: Thursday, May 9, 2024 8:59 AM > To: intel-gfx@lists.freedesktop.org > Cc: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>; > Kandpal, Suraj <suraj.kandpal@intel.com> > Subject: [PATCH] drm/i915/pps: Disable DPLS_GATING around pps sequence > > Disable bit 29 of SCLKGATE_DIS register around pps sequence when we turn > panel power on. > > --v2 > -Squash two commit together [Jani] > -Use IS_DISPLAY_VER [Jani] > -Fix multiline comment [Jani] > > --v3 > -Define register in a more appropriate place [Mitul] > > Bspec: 49304 > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_pps.c | 12 ++++++++++++ > drivers/gpu/drm/i915/i915_reg.h | 4 ++++ > 2 files changed, 16 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_pps.c > b/drivers/gpu/drm/i915/display/intel_pps.c > index 0ccbf9a85914..d774aeb1673e 100644 > --- a/drivers/gpu/drm/i915/display/intel_pps.c > +++ b/drivers/gpu/drm/i915/display/intel_pps.c > @@ -948,6 +948,14 @@ void intel_pps_on_unlocked(struct intel_dp *intel_dp) > intel_de_posting_read(dev_priv, pp_ctrl_reg); > } > > + /* Hi Suraj, > + * WA: 16023567976 --------------------^^^^^^^^ is not the correct WA number. Please mention the linage number here. > + * Disable DPLS gating around power sequence. > + */ > + if (IS_DISPLAY_VER(dev_priv, 12, 14)) Also, I did not find the link to the display version 12 for this WA. I could see the target display version for this WA is 13.00 and 14.00 as a permanent WA for now. Dnyaneshwar > + intel_de_rmw(dev_priv, SCLKGATE_DIS, > + 0, DPLS_GATING_DISABLE); > + > pp |= PANEL_POWER_ON; > if (!IS_IRONLAKE(dev_priv)) > pp |= PANEL_POWER_RESET; > @@ -958,6 +966,10 @@ void intel_pps_on_unlocked(struct intel_dp *intel_dp) > wait_panel_on(intel_dp); > intel_dp->pps.last_power_on = jiffies; > > + if (IS_DISPLAY_VER(dev_priv, 12, 14)) > + intel_de_rmw(dev_priv, SCLKGATE_DIS, > + DPLS_GATING_DISABLE, 0); > + > if (IS_IRONLAKE(dev_priv)) { > pp |= PANEL_POWER_RESET; /* restore panel reset bit */ > intel_de_write(dev_priv, pp_ctrl_reg, pp); diff --git > a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index > 5670eee4a498..4cc82360298b 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -5151,6 +5151,10 @@ enum skl_power_gate { > > #define _MMIO_PLANE_GAMC(plane, i, a, b) _MMIO(_PIPE(plane, a, b) + (i) * 4) > > +/* SCLKGATE_DIS */ > +#define SCLKGATE_DIS _MMIO(0xc2020) > +#define DPLS_GATING_DISABLE REG_BIT(29) > + > /* Plane CSC Registers */ > #define _PLANE_CSC_RY_GY_1_A 0x70210 > #define _PLANE_CSC_RY_GY_2_A 0x70310 > -- > 2.43.2
> -----Original Message----- > From: Bhadane, Dnyaneshwar <dnyaneshwar.bhadane@intel.com> > Sent: Monday, July 29, 2024 7:24 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel-gfx@lists.freedesktop.org > Cc: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>; > Kandpal, Suraj <suraj.kandpal@intel.com> > Subject: RE: [PATCH] drm/i915/pps: Disable DPLS_GATING around pps sequence > > > > > -----Original Message----- > > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of > > Suraj Kandpal > > Sent: Thursday, May 9, 2024 8:59 AM > > To: intel-gfx@lists.freedesktop.org > > Cc: Golani, Mitulkumar Ajitkumar > > <mitulkumar.ajitkumar.golani@intel.com>; > > Kandpal, Suraj <suraj.kandpal@intel.com> > > Subject: [PATCH] drm/i915/pps: Disable DPLS_GATING around pps sequence > > > > Disable bit 29 of SCLKGATE_DIS register around pps sequence when we > > turn panel power on. > > > > --v2 > > -Squash two commit together [Jani] > > -Use IS_DISPLAY_VER [Jani] > > -Fix multiline comment [Jani] > > > > --v3 > > -Define register in a more appropriate place [Mitul] > > > > Bspec: 49304 > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_pps.c | 12 ++++++++++++ > > drivers/gpu/drm/i915/i915_reg.h | 4 ++++ > > 2 files changed, 16 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_pps.c > > b/drivers/gpu/drm/i915/display/intel_pps.c > > index 0ccbf9a85914..d774aeb1673e 100644 > > --- a/drivers/gpu/drm/i915/display/intel_pps.c > > +++ b/drivers/gpu/drm/i915/display/intel_pps.c > > @@ -948,6 +948,14 @@ void intel_pps_on_unlocked(struct intel_dp > *intel_dp) > > intel_de_posting_read(dev_priv, pp_ctrl_reg); > > } > > > > + /* > Hi Suraj, > > + * WA: 16023567976 > --------------------^^^^^^^^ is not the correct WA number. > Please mention the linage number here. > > > + * Disable DPLS gating around power sequence. > > + */ > > + if (IS_DISPLAY_VER(dev_priv, 12, 14)) > > Also, I did not find the link to the display version 12 for this WA. > I could see the target display version for this WA is 13.00 and 14.00 as a > permanent WA for now. > Not sure how you checked this but if you go to the WA number it clearly states Family affected as : Display 12,13,14 Regards, Suraj Kandpal > Dnyaneshwar > > > + intel_de_rmw(dev_priv, SCLKGATE_DIS, > > + 0, DPLS_GATING_DISABLE); > > + > > pp |= PANEL_POWER_ON; > > if (!IS_IRONLAKE(dev_priv)) > > pp |= PANEL_POWER_RESET; > > @@ -958,6 +966,10 @@ void intel_pps_on_unlocked(struct intel_dp > *intel_dp) > > wait_panel_on(intel_dp); > > intel_dp->pps.last_power_on = jiffies; > > > > + if (IS_DISPLAY_VER(dev_priv, 12, 14)) > > + intel_de_rmw(dev_priv, SCLKGATE_DIS, > > + DPLS_GATING_DISABLE, 0); > > + > > if (IS_IRONLAKE(dev_priv)) { > > pp |= PANEL_POWER_RESET; /* restore panel reset bit */ > > intel_de_write(dev_priv, pp_ctrl_reg, pp); diff --git > > a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > index 5670eee4a498..4cc82360298b 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -5151,6 +5151,10 @@ enum skl_power_gate { > > > > #define _MMIO_PLANE_GAMC(plane, i, a, b) _MMIO(_PIPE(plane, a, b) + > > (i) * 4) > > > > +/* SCLKGATE_DIS */ > > +#define SCLKGATE_DIS _MMIO(0xc2020) > > +#define DPLS_GATING_DISABLE REG_BIT(29) > > + > > /* Plane CSC Registers */ > > #define _PLANE_CSC_RY_GY_1_A 0x70210 > > #define _PLANE_CSC_RY_GY_2_A 0x70310 > > -- > > 2.43.2
> -----Original Message----- > From: Kandpal, Suraj <suraj.kandpal@intel.com> > Sent: Tuesday, July 30, 2024 9:39 AM > To: Bhadane, Dnyaneshwar <dnyaneshwar.bhadane@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com> > Subject: RE: [PATCH] drm/i915/pps: Disable DPLS_GATING around pps sequence > > > > > -----Original Message----- > > From: Bhadane, Dnyaneshwar <dnyaneshwar.bhadane@intel.com> > > Sent: Monday, July 29, 2024 7:24 PM > > To: Kandpal, Suraj <suraj.kandpal@intel.com>; > > intel-gfx@lists.freedesktop.org > > Cc: Golani, Mitulkumar Ajitkumar > > <mitulkumar.ajitkumar.golani@intel.com>; > > Kandpal, Suraj <suraj.kandpal@intel.com> > > Subject: RE: [PATCH] drm/i915/pps: Disable DPLS_GATING around pps > > sequence > > > > > > > > > -----Original Message----- > > > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf > > > Of Suraj Kandpal > > > Sent: Thursday, May 9, 2024 8:59 AM > > > To: intel-gfx@lists.freedesktop.org > > > Cc: Golani, Mitulkumar Ajitkumar > > > <mitulkumar.ajitkumar.golani@intel.com>; > > > Kandpal, Suraj <suraj.kandpal@intel.com> > > > Subject: [PATCH] drm/i915/pps: Disable DPLS_GATING around pps > > > sequence > > > > > > Disable bit 29 of SCLKGATE_DIS register around pps sequence when we > > > turn panel power on. > > > > > > --v2 > > > -Squash two commit together [Jani] > > > -Use IS_DISPLAY_VER [Jani] > > > -Fix multiline comment [Jani] > > > > > > --v3 > > > -Define register in a more appropriate place [Mitul] > > > > > > Bspec: 49304 > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_pps.c | 12 ++++++++++++ > > > drivers/gpu/drm/i915/i915_reg.h | 4 ++++ > > > 2 files changed, 16 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_pps.c > > > b/drivers/gpu/drm/i915/display/intel_pps.c > > > index 0ccbf9a85914..d774aeb1673e 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_pps.c > > > +++ b/drivers/gpu/drm/i915/display/intel_pps.c > > > @@ -948,6 +948,14 @@ void intel_pps_on_unlocked(struct intel_dp > > *intel_dp) > > > intel_de_posting_read(dev_priv, pp_ctrl_reg); > > > } > > > > > > + /* > > Hi Suraj, > > > + * WA: 16023567976 > > --------------------^^^^^^^^ is not the correct WA number. > > Please mention the linage number here. > > > > > + * Disable DPLS gating around power sequence. > > > + */ > > > + if (IS_DISPLAY_VER(dev_priv, 12, 14)) > > > > Also, I did not find the link to the display version 12 for this WA. > > I could see the target display version for this WA is 13.00 and 14.00 > > as a permanent WA for now. > > > > Not sure how you checked this but if you go to the WA number it clearly states > Family affected as : Display 12,13,14 Please check "release" attribute and WA's under the same lineage(in this case 22019252566). You should implement only the WA marked as permanent. If you think required platform is missing, please comment in the issue. Dnyaneshwar. > > Regards, > Suraj Kandpal > > Dnyaneshwar > > > > > + intel_de_rmw(dev_priv, SCLKGATE_DIS, > > > + 0, DPLS_GATING_DISABLE); > > > + > > > pp |= PANEL_POWER_ON; > > > if (!IS_IRONLAKE(dev_priv)) > > > pp |= PANEL_POWER_RESET; > > > @@ -958,6 +966,10 @@ void intel_pps_on_unlocked(struct intel_dp > > *intel_dp) > > > wait_panel_on(intel_dp); > > > intel_dp->pps.last_power_on = jiffies; > > > > > > + if (IS_DISPLAY_VER(dev_priv, 12, 14)) > > > + intel_de_rmw(dev_priv, SCLKGATE_DIS, > > > + DPLS_GATING_DISABLE, 0); > > > + > > > if (IS_IRONLAKE(dev_priv)) { > > > pp |= PANEL_POWER_RESET; /* restore panel reset bit */ > > > intel_de_write(dev_priv, pp_ctrl_reg, pp); diff --git > > > a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > > index 5670eee4a498..4cc82360298b 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -5151,6 +5151,10 @@ enum skl_power_gate { > > > > > > #define _MMIO_PLANE_GAMC(plane, i, a, b) _MMIO(_PIPE(plane, a, b) > > > + > > > (i) * 4) > > > > > > +/* SCLKGATE_DIS */ > > > +#define SCLKGATE_DIS _MMIO(0xc2020) > > > +#define DPLS_GATING_DISABLE REG_BIT(29) > > > + > > > /* Plane CSC Registers */ > > > #define _PLANE_CSC_RY_GY_1_A 0x70210 > > > #define _PLANE_CSC_RY_GY_2_A 0x70310 > > > -- > > > 2.43.2
diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c index 0ccbf9a85914..d774aeb1673e 100644 --- a/drivers/gpu/drm/i915/display/intel_pps.c +++ b/drivers/gpu/drm/i915/display/intel_pps.c @@ -948,6 +948,14 @@ void intel_pps_on_unlocked(struct intel_dp *intel_dp) intel_de_posting_read(dev_priv, pp_ctrl_reg); } + /* + * WA: 16023567976 + * Disable DPLS gating around power sequence. + */ + if (IS_DISPLAY_VER(dev_priv, 12, 14)) + intel_de_rmw(dev_priv, SCLKGATE_DIS, + 0, DPLS_GATING_DISABLE); + pp |= PANEL_POWER_ON; if (!IS_IRONLAKE(dev_priv)) pp |= PANEL_POWER_RESET; @@ -958,6 +966,10 @@ void intel_pps_on_unlocked(struct intel_dp *intel_dp) wait_panel_on(intel_dp); intel_dp->pps.last_power_on = jiffies; + if (IS_DISPLAY_VER(dev_priv, 12, 14)) + intel_de_rmw(dev_priv, SCLKGATE_DIS, + DPLS_GATING_DISABLE, 0); + if (IS_IRONLAKE(dev_priv)) { pp |= PANEL_POWER_RESET; /* restore panel reset bit */ intel_de_write(dev_priv, pp_ctrl_reg, pp); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 5670eee4a498..4cc82360298b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5151,6 +5151,10 @@ enum skl_power_gate { #define _MMIO_PLANE_GAMC(plane, i, a, b) _MMIO(_PIPE(plane, a, b) + (i) * 4) +/* SCLKGATE_DIS */ +#define SCLKGATE_DIS _MMIO(0xc2020) +#define DPLS_GATING_DISABLE REG_BIT(29) + /* Plane CSC Registers */ #define _PLANE_CSC_RY_GY_1_A 0x70210 #define _PLANE_CSC_RY_GY_2_A 0x70310
Disable bit 29 of SCLKGATE_DIS register around pps sequence when we turn panel power on. --v2 -Squash two commit together [Jani] -Use IS_DISPLAY_VER [Jani] -Fix multiline comment [Jani] --v3 -Define register in a more appropriate place [Mitul] Bspec: 49304 Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> --- drivers/gpu/drm/i915/display/intel_pps.c | 12 ++++++++++++ drivers/gpu/drm/i915/i915_reg.h | 4 ++++ 2 files changed, 16 insertions(+)