| Message ID | 20210622093424.886133-1-tejaskumarx.surendrakumar.upadhyay@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [V4] drm/i915/gen11: Disable cursor clock gating in HDR mode | expand |
On Tue, Jun 22, 2021 at 03:04:24PM +0530, Tejas Upadhyay wrote: > Display underrun in HDR mode when cursor is enabled. > RTL fix will be implemented CLKGATE_DIS_PSL_A bit 28-46520h. > As per W/A 1604331009, Disable cursor clock gating in HDR mode. > > Bspec : 33451 > > Changes since V3: > - Disable WA when not in HDR mode or cursor plane not active - Ville > - Extract required args from crtc_state - Ville > - Create HDR mode API using bdw_set_pipemisc ref - Ville > - Tested with HDR video as well full setmode, WA applies and disables > Changes since V2: > - Made it general gen11 WA > - Removed WA needed check > - Added cursor plane active check > - Once WA enable, software will not disable > Changes since V1: > - Modified way CLKGATE_DIS_PSL bit 28 was modified > > Cc: Souza Jose <jose.souza@intel.com> > Signed-off-by: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 27 ++++++++++++++++++++ > drivers/gpu/drm/i915/i915_reg.h | 5 ++++ > 2 files changed, 32 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 6be1b31af07b..e1ea03b918df 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -358,6 +358,13 @@ static void intel_update_czclk(struct drm_i915_private *dev_priv) > dev_priv->czclk_freq); > } > > +static bool > +is_hdr_mode(const struct intel_crtc_state *crtc_state) > +{ > + return (crtc_state->active_planes & ~(icl_hdr_plane_mask() | > + BIT(PLANE_CURSOR))) == 0; > +} Please use this in bdw_set_pipemisc() as well. This could be a separate prep patch actually. > + > /* WA Display #0827: Gen9:all */ > static void > skl_wa_827(struct drm_i915_private *dev_priv, enum pipe pipe, bool enable) > @@ -383,6 +390,23 @@ icl_wa_scalerclkgating(struct drm_i915_private *dev_priv, enum pipe pipe, > intel_de_read(dev_priv, CLKGATE_DIS_PSL(pipe)) & ~DPFR_GATING_DIS); > } > > +/* Wa_1604331009:icl,jsl,ehl */ > + static void > +icl_wa_cursorclkgating(const struct intel_crtc_state *crtc_state) > +{ > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > + > + if (is_hdr_mode(crtc_state) && > + crtc_state->active_planes & BIT(PLANE_CURSOR) && > + IS_GEN(dev_priv, 11)) > + intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe), > + CURSOR_GATING_DIS, CURSOR_GATING_DIS); > + else > + intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe), > + CURSOR_GATING_DIS, 0); > +} > + > static bool > is_trans_port_sync_slave(const struct intel_crtc_state *crtc_state) > { > @@ -2939,6 +2963,9 @@ static void intel_pre_plane_update(struct intel_atomic_state *state, > needs_scalerclk_wa(new_crtc_state)) > icl_wa_scalerclkgating(dev_priv, pipe, true); > > + /* Wa_1604331009:icl,jsl,ehl */ > + icl_wa_cursorclkgating(new_crtc_state); This looks a bit wrong. We shouldn't turn the clock gating back on until after HDR mode has been disabled. So please model this after skl_wa_827() and icl_wa_scalerclkgating() so that a) the ordering is correct, and b) the code between all three w/as looks consistent. > + > /* > * Vblank time updates from the shadow to live plane control register > * are blocked if the memory self-refresh mode is active at that > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index c857fafb8a30..703d708c773e 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -4235,6 +4235,11 @@ enum { > #define INF_UNIT_LEVEL_CLKGATE _MMIO(0x9560) > #define CGPSF_CLKGATE_DIS (1 << 3) > > +/* > + * GEN11 clock gating regs > + */ > +#define CURSOR_GATING_DIS BIT(28) This looks misplaced. It should be next to the other bits of whatever register this is. Also pls use REG_BIT() instead of BIT(). > + > /* > * Display engine regs > */ > -- > 2.31.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: 01 September 2021 19:19 > To: Surendrakumar Upadhyay, TejaskumarX > <tejaskumarx.surendrakumar.upadhyay@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock > gating in HDR mode > > On Tue, Jun 22, 2021 at 03:04:24PM +0530, Tejas Upadhyay wrote: > > Display underrun in HDR mode when cursor is enabled. > > RTL fix will be implemented CLKGATE_DIS_PSL_A bit 28-46520h. > > As per W/A 1604331009, Disable cursor clock gating in HDR mode. > > > > Bspec : 33451 > > > > Changes since V3: > > - Disable WA when not in HDR mode or cursor plane not active - Ville > > - Extract required args from crtc_state - Ville > > - Create HDR mode API using bdw_set_pipemisc ref - Ville > > - Tested with HDR video as well full setmode, WA applies and > disables > > Changes since V2: > > - Made it general gen11 WA > > - Removed WA needed check > > - Added cursor plane active check > > - Once WA enable, software will not disable Changes since V1: > > - Modified way CLKGATE_DIS_PSL bit 28 was modified > > > > Cc: Souza Jose <jose.souza@intel.com> > > Signed-off-by: Tejas Upadhyay > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 27 ++++++++++++++++++++ > > drivers/gpu/drm/i915/i915_reg.h | 5 ++++ > > 2 files changed, 32 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index 6be1b31af07b..e1ea03b918df 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -358,6 +358,13 @@ static void intel_update_czclk(struct > drm_i915_private *dev_priv) > > dev_priv->czclk_freq); > > } > > > > +static bool > > +is_hdr_mode(const struct intel_crtc_state *crtc_state) { > > + return (crtc_state->active_planes & ~(icl_hdr_plane_mask() | > > + BIT(PLANE_CURSOR))) == 0; > > +} > > Please use this in bdw_set_pipemisc() as well. This could be a separate prep > patch actually. > > > + > > /* WA Display #0827: Gen9:all */ > > static void > > skl_wa_827(struct drm_i915_private *dev_priv, enum pipe pipe, bool > > enable) @@ -383,6 +390,23 @@ icl_wa_scalerclkgating(struct > drm_i915_private *dev_priv, enum pipe pipe, > > intel_de_read(dev_priv, CLKGATE_DIS_PSL(pipe)) & > > ~DPFR_GATING_DIS); } > > > > +/* Wa_1604331009:icl,jsl,ehl */ > > + static void > > +icl_wa_cursorclkgating(const struct intel_crtc_state *crtc_state) { > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > + > > + if (is_hdr_mode(crtc_state) && > > + crtc_state->active_planes & BIT(PLANE_CURSOR) && > > + IS_GEN(dev_priv, 11)) > > + intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe), > > + CURSOR_GATING_DIS, CURSOR_GATING_DIS); > > + else > > + intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe), > > + CURSOR_GATING_DIS, 0); > > +} > > + > > static bool > > is_trans_port_sync_slave(const struct intel_crtc_state *crtc_state) > > { @@ -2939,6 +2963,9 @@ static void intel_pre_plane_update(struct > > intel_atomic_state *state, > > needs_scalerclk_wa(new_crtc_state)) > > icl_wa_scalerclkgating(dev_priv, pipe, true); > > > > + /* Wa_1604331009:icl,jsl,ehl */ > > + icl_wa_cursorclkgating(new_crtc_state); > > This looks a bit wrong. We shouldn't turn the clock gating back on until after > HDR mode has been disabled. > > So please model this after skl_wa_827() and icl_wa_scalerclkgating() so that > a) the ordering is correct, and b) the code between all three w/as looks > consistent. I did not get what you are suggesting here. Can you please put psudo? Currently as far as I see icl_wa_cursorclkgating is already modelled after skl_wa_827() and icl_wa_scalerclkgating(). Are referring same Or something else? Thanks, Tejas > > > + > > /* > > * Vblank time updates from the shadow to live plane control register > > * are blocked if the memory self-refresh mode is active at that > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h index c857fafb8a30..703d708c773e > > 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -4235,6 +4235,11 @@ enum { > > #define INF_UNIT_LEVEL_CLKGATE _MMIO(0x9560) > > #define CGPSF_CLKGATE_DIS (1 << 3) > > > > +/* > > + * GEN11 clock gating regs > > + */ > > +#define CURSOR_GATING_DIS BIT(28) > > This looks misplaced. It should be next to the other bits of whatever register > this is. Also pls use REG_BIT() instead of BIT(). > > > + > > /* > > * Display engine regs > > */ > > -- > > 2.31.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel
On Thu, Sep 02, 2021 at 11:07:06AM +0000, Surendrakumar Upadhyay, TejaskumarX wrote: > > > > -----Original Message----- > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Sent: 01 September 2021 19:19 > > To: Surendrakumar Upadhyay, TejaskumarX > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > Cc: intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock > > gating in HDR mode > > > > On Tue, Jun 22, 2021 at 03:04:24PM +0530, Tejas Upadhyay wrote: > > > Display underrun in HDR mode when cursor is enabled. > > > RTL fix will be implemented CLKGATE_DIS_PSL_A bit 28-46520h. > > > As per W/A 1604331009, Disable cursor clock gating in HDR mode. > > > > > > Bspec : 33451 > > > > > > Changes since V3: > > > - Disable WA when not in HDR mode or cursor plane not active - Ville > > > - Extract required args from crtc_state - Ville > > > - Create HDR mode API using bdw_set_pipemisc ref - Ville > > > - Tested with HDR video as well full setmode, WA applies and > > disables > > > Changes since V2: > > > - Made it general gen11 WA > > > - Removed WA needed check > > > - Added cursor plane active check > > > - Once WA enable, software will not disable Changes since V1: > > > - Modified way CLKGATE_DIS_PSL bit 28 was modified > > > > > > Cc: Souza Jose <jose.souza@intel.com> > > > Signed-off-by: Tejas Upadhyay > > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_display.c | 27 ++++++++++++++++++++ > > > drivers/gpu/drm/i915/i915_reg.h | 5 ++++ > > > 2 files changed, 32 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > index 6be1b31af07b..e1ea03b918df 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > @@ -358,6 +358,13 @@ static void intel_update_czclk(struct > > drm_i915_private *dev_priv) > > > dev_priv->czclk_freq); > > > } > > > > > > +static bool > > > +is_hdr_mode(const struct intel_crtc_state *crtc_state) { > > > + return (crtc_state->active_planes & ~(icl_hdr_plane_mask() | > > > + BIT(PLANE_CURSOR))) == 0; > > > +} > > > > Please use this in bdw_set_pipemisc() as well. This could be a separate prep > > patch actually. > > > > > + > > > /* WA Display #0827: Gen9:all */ > > > static void > > > skl_wa_827(struct drm_i915_private *dev_priv, enum pipe pipe, bool > > > enable) @@ -383,6 +390,23 @@ icl_wa_scalerclkgating(struct > > drm_i915_private *dev_priv, enum pipe pipe, > > > intel_de_read(dev_priv, CLKGATE_DIS_PSL(pipe)) & > > > ~DPFR_GATING_DIS); } > > > > > > +/* Wa_1604331009:icl,jsl,ehl */ > > > + static void > > > +icl_wa_cursorclkgating(const struct intel_crtc_state *crtc_state) { > > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > > + > > > + if (is_hdr_mode(crtc_state) && > > > + crtc_state->active_planes & BIT(PLANE_CURSOR) && > > > + IS_GEN(dev_priv, 11)) > > > + intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe), > > > + CURSOR_GATING_DIS, CURSOR_GATING_DIS); > > > + else > > > + intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe), > > > + CURSOR_GATING_DIS, 0); > > > +} > > > + > > > static bool > > > is_trans_port_sync_slave(const struct intel_crtc_state *crtc_state) > > > { @@ -2939,6 +2963,9 @@ static void intel_pre_plane_update(struct > > > intel_atomic_state *state, > > > needs_scalerclk_wa(new_crtc_state)) > > > icl_wa_scalerclkgating(dev_priv, pipe, true); > > > > > > + /* Wa_1604331009:icl,jsl,ehl */ > > > + icl_wa_cursorclkgating(new_crtc_state); > > > > This looks a bit wrong. We shouldn't turn the clock gating back on until after > > HDR mode has been disabled. > > > > So please model this after skl_wa_827() and icl_wa_scalerclkgating() so that > > a) the ordering is correct, and b) the code between all three w/as looks > > consistent. > > I did not get what you are suggesting here. Can you please put psudo? Currently as far as I see icl_wa_cursorclkgating is already modelled after skl_wa_827() and icl_wa_scalerclkgating(). Are referring same > Or something else? It should look something like: intel_pre_plane_update() { if (!needs_cursorclk_wa(old_crtc_state) && needs_cursorclk_wa(new_crtc_state)) icl_wa_cursorclkgating(..., true); } intel_post_plane_update() { if (needs_cursorclk_wa(old_crtc_state) && !needs_cursorclk_wa(new_crtc_state)) icl_wa_cursorclkgating(..., false); }
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: 02 September 2021 18:29 > To: Surendrakumar Upadhyay, TejaskumarX > <tejaskumarx.surendrakumar.upadhyay@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock > gating in HDR mode > > On Thu, Sep 02, 2021 at 11:07:06AM +0000, Surendrakumar Upadhyay, > TejaskumarX wrote: > > > > > > > -----Original Message----- > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Sent: 01 September 2021 19:19 > > > To: Surendrakumar Upadhyay, TejaskumarX > > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > > Cc: intel-gfx@lists.freedesktop.org > > > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor > > > clock gating in HDR mode > > > > > > On Tue, Jun 22, 2021 at 03:04:24PM +0530, Tejas Upadhyay wrote: > > > > Display underrun in HDR mode when cursor is enabled. > > > > RTL fix will be implemented CLKGATE_DIS_PSL_A bit 28-46520h. > > > > As per W/A 1604331009, Disable cursor clock gating in HDR mode. > > > > > > > > Bspec : 33451 > > > > > > > > Changes since V3: > > > > - Disable WA when not in HDR mode or cursor plane not active - Ville > > > > - Extract required args from crtc_state - Ville > > > > - Create HDR mode API using bdw_set_pipemisc ref - Ville > > > > - Tested with HDR video as well full setmode, WA applies and > > > disables > > > > Changes since V2: > > > > - Made it general gen11 WA > > > > - Removed WA needed check > > > > - Added cursor plane active check > > > > - Once WA enable, software will not disable Changes since V1: > > > > - Modified way CLKGATE_DIS_PSL bit 28 was modified > > > > > > > > Cc: Souza Jose <jose.souza@intel.com> > > > > Signed-off-by: Tejas Upadhyay > > > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/display/intel_display.c | 27 > ++++++++++++++++++++ > > > > drivers/gpu/drm/i915/i915_reg.h | 5 ++++ > > > > 2 files changed, 32 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > > index 6be1b31af07b..e1ea03b918df 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > @@ -358,6 +358,13 @@ static void intel_update_czclk(struct > > > drm_i915_private *dev_priv) > > > > dev_priv->czclk_freq); > > > > } > > > > > > > > +static bool > > > > +is_hdr_mode(const struct intel_crtc_state *crtc_state) { > > > > + return (crtc_state->active_planes & ~(icl_hdr_plane_mask() | > > > > + BIT(PLANE_CURSOR))) == 0; > > > > +} > > > > > > Please use this in bdw_set_pipemisc() as well. This could be a > > > separate prep patch actually. > > > > > > > + > > > > /* WA Display #0827: Gen9:all */ > > > > static void > > > > skl_wa_827(struct drm_i915_private *dev_priv, enum pipe pipe, > > > > bool > > > > enable) @@ -383,6 +390,23 @@ icl_wa_scalerclkgating(struct > > > drm_i915_private *dev_priv, enum pipe pipe, > > > > intel_de_read(dev_priv, CLKGATE_DIS_PSL(pipe)) & > > > > ~DPFR_GATING_DIS); } > > > > > > > > +/* Wa_1604331009:icl,jsl,ehl */ > > > > + static void > > > > +icl_wa_cursorclkgating(const struct intel_crtc_state *crtc_state) { > > > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > > > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > > > + > > > > + if (is_hdr_mode(crtc_state) && > > > > + crtc_state->active_planes & BIT(PLANE_CURSOR) && > > > > + IS_GEN(dev_priv, 11)) > > > > + intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe), > > > > + CURSOR_GATING_DIS, CURSOR_GATING_DIS); > > > > + else > > > > + intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe), > > > > + CURSOR_GATING_DIS, 0); > > > > +} > > > > + > > > > static bool > > > > is_trans_port_sync_slave(const struct intel_crtc_state > > > > *crtc_state) { @@ -2939,6 +2963,9 @@ static void > > > > intel_pre_plane_update(struct intel_atomic_state *state, > > > > needs_scalerclk_wa(new_crtc_state)) > > > > icl_wa_scalerclkgating(dev_priv, pipe, true); > > > > > > > > + /* Wa_1604331009:icl,jsl,ehl */ > > > > + icl_wa_cursorclkgating(new_crtc_state); > > > > > > This looks a bit wrong. We shouldn't turn the clock gating back on > > > until after HDR mode has been disabled. > > > > > > So please model this after skl_wa_827() and icl_wa_scalerclkgating() > > > so that > > > a) the ordering is correct, and b) the code between all three w/as > > > looks consistent. > > > > I did not get what you are suggesting here. Can you please put psudo? > > Currently as far as I see icl_wa_cursorclkgating is already modelled after > skl_wa_827() and icl_wa_scalerclkgating(). Are referring same Or something > else? > > It should look something like: > > intel_pre_plane_update() > { > if (!needs_cursorclk_wa(old_crtc_state) && > needs_cursorclk_wa(new_crtc_state)) > icl_wa_cursorclkgating(..., true); > } > > intel_post_plane_update() > { > if (needs_cursorclk_wa(old_crtc_state) && > !needs_cursorclk_wa(new_crtc_state)) > icl_wa_cursorclkgating(..., false); > } > Tejas : In the previous version it was done this way only. But after review comments from Jose I had to change it. See "Changes since V2" section. Also you can check in previous versions of patch. > > -- > Ville Syrjälä > Intel
+ Hariom > -----Original Message----- > From: Surendrakumar Upadhyay, TejaskumarX > Sent: 02 September 2021 18:34 > To: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: intel-gfx@lists.freedesktop.org; Souza, Jose <jose.souza@intel.com> > Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock > gating in HDR mode > > > > > -----Original Message----- > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Sent: 02 September 2021 18:29 > > To: Surendrakumar Upadhyay, TejaskumarX > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > Cc: intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor > > clock gating in HDR mode > > > > On Thu, Sep 02, 2021 at 11:07:06AM +0000, Surendrakumar Upadhyay, > > TejaskumarX wrote: > > > > > > > > > > -----Original Message----- > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Sent: 01 September 2021 19:19 > > > > To: Surendrakumar Upadhyay, TejaskumarX > > > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > > > Cc: intel-gfx@lists.freedesktop.org > > > > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor > > > > clock gating in HDR mode > > > > > > > > On Tue, Jun 22, 2021 at 03:04:24PM +0530, Tejas Upadhyay wrote: > > > > > Display underrun in HDR mode when cursor is enabled. > > > > > RTL fix will be implemented CLKGATE_DIS_PSL_A bit 28-46520h. > > > > > As per W/A 1604331009, Disable cursor clock gating in HDR mode. > > > > > > > > > > Bspec : 33451 > > > > > > > > > > Changes since V3: > > > > > - Disable WA when not in HDR mode or cursor plane not active - Ville > > > > > - Extract required args from crtc_state - Ville > > > > > - Create HDR mode API using bdw_set_pipemisc ref - Ville > > > > > - Tested with HDR video as well full setmode, WA applies and > > > > disables > > > > > Changes since V2: > > > > > - Made it general gen11 WA > > > > > - Removed WA needed check > > > > > - Added cursor plane active check > > > > > - Once WA enable, software will not disable Changes since V1: > > > > > - Modified way CLKGATE_DIS_PSL bit 28 was modified > > > > > > > > > > Cc: Souza Jose <jose.souza@intel.com> > > > > > Signed-off-by: Tejas Upadhyay > > > > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/display/intel_display.c | 27 > > ++++++++++++++++++++ > > > > > drivers/gpu/drm/i915/i915_reg.h | 5 ++++ > > > > > 2 files changed, 32 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > > > index 6be1b31af07b..e1ea03b918df 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > > @@ -358,6 +358,13 @@ static void intel_update_czclk(struct > > > > drm_i915_private *dev_priv) > > > > > dev_priv->czclk_freq); > > > > > } > > > > > > > > > > +static bool > > > > > +is_hdr_mode(const struct intel_crtc_state *crtc_state) { > > > > > + return (crtc_state->active_planes & ~(icl_hdr_plane_mask() | > > > > > + BIT(PLANE_CURSOR))) == 0; > > > > > +} > > > > > > > > Please use this in bdw_set_pipemisc() as well. This could be a > > > > separate prep patch actually. > > > > > > > > > + > > > > > /* WA Display #0827: Gen9:all */ static void > > > > > skl_wa_827(struct drm_i915_private *dev_priv, enum pipe pipe, > > > > > bool > > > > > enable) @@ -383,6 +390,23 @@ icl_wa_scalerclkgating(struct > > > > drm_i915_private *dev_priv, enum pipe pipe, > > > > > intel_de_read(dev_priv, CLKGATE_DIS_PSL(pipe)) > > > > > & ~DPFR_GATING_DIS); } > > > > > > > > > > +/* Wa_1604331009:icl,jsl,ehl */ > > > > > + static void > > > > > +icl_wa_cursorclkgating(const struct intel_crtc_state *crtc_state) { > > > > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > > > > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > > > > + > > > > > + if (is_hdr_mode(crtc_state) && > > > > > + crtc_state->active_planes & BIT(PLANE_CURSOR) && > > > > > + IS_GEN(dev_priv, 11)) > > > > > + intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc- > >pipe), > > > > > + CURSOR_GATING_DIS, > CURSOR_GATING_DIS); > > > > > + else > > > > > + intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc- > >pipe), > > > > > + CURSOR_GATING_DIS, 0); > > > > > +} > > > > > + > > > > > static bool > > > > > is_trans_port_sync_slave(const struct intel_crtc_state > > > > > *crtc_state) { @@ -2939,6 +2963,9 @@ static void > > > > > intel_pre_plane_update(struct intel_atomic_state *state, > > > > > needs_scalerclk_wa(new_crtc_state)) > > > > > icl_wa_scalerclkgating(dev_priv, pipe, true); > > > > > > > > > > + /* Wa_1604331009:icl,jsl,ehl */ > > > > > + icl_wa_cursorclkgating(new_crtc_state); > > > > > > > > This looks a bit wrong. We shouldn't turn the clock gating back on > > > > until after HDR mode has been disabled. > > > > > > > > So please model this after skl_wa_827() and > > > > icl_wa_scalerclkgating() so that > > > > a) the ordering is correct, and b) the code between all three w/as > > > > looks consistent. > > > > > > I did not get what you are suggesting here. Can you please put psudo? > > > Currently as far as I see icl_wa_cursorclkgating is already modelled > > > after > > skl_wa_827() and icl_wa_scalerclkgating(). Are referring same Or > > something else? > > > > It should look something like: > > > > intel_pre_plane_update() > > { > > if (!needs_cursorclk_wa(old_crtc_state) && > > needs_cursorclk_wa(new_crtc_state)) > > icl_wa_cursorclkgating(..., true); > > } > > > > intel_post_plane_update() > > { > > if (needs_cursorclk_wa(old_crtc_state) && > > !needs_cursorclk_wa(new_crtc_state)) > > icl_wa_cursorclkgating(..., false); > > } > > > > Tejas : In the previous version it was done this way only. But after review > comments from Jose I had to change it. See "Changes since V2" section. Also > you can check in previous versions of patch. > > > > > -- > > Ville Syrjälä > > Intel
Hi Ville/Jose, I hope you both discussed as in next version I will incorporate all changes as per Ville's final suggestion. Please let me know if you guys think otherwise. Thanks, Tejas > -----Original Message----- > From: Surendrakumar Upadhyay, TejaskumarX > Sent: 02 September 2021 18:37 > To: 'Ville Syrjälä' <ville.syrjala@linux.intel.com> > Cc: 'intel-gfx@lists.freedesktop.org' <intel-gfx@lists.freedesktop.org>; Souza, > Jose <jose.souza@intel.com>; Pandey, Hariom <hariom.pandey@intel.com> > Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock > gating in HDR mode > > + Hariom > > > -----Original Message----- > > From: Surendrakumar Upadhyay, TejaskumarX > > Sent: 02 September 2021 18:34 > > To: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: intel-gfx@lists.freedesktop.org; Souza, Jose > > <jose.souza@intel.com> > > Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor > > clock gating in HDR mode > > > > > > > > > -----Original Message----- > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Sent: 02 September 2021 18:29 > > > To: Surendrakumar Upadhyay, TejaskumarX > > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > > Cc: intel-gfx@lists.freedesktop.org > > > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor > > > clock gating in HDR mode > > > > > > On Thu, Sep 02, 2021 at 11:07:06AM +0000, Surendrakumar Upadhyay, > > > TejaskumarX wrote: > > > > > > > > > > > > > -----Original Message----- > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > Sent: 01 September 2021 19:19 > > > > > To: Surendrakumar Upadhyay, TejaskumarX > > > > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > > > > Cc: intel-gfx@lists.freedesktop.org > > > > > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable > > > > > cursor clock gating in HDR mode > > > > > > > > > > On Tue, Jun 22, 2021 at 03:04:24PM +0530, Tejas Upadhyay wrote: > > > > > > Display underrun in HDR mode when cursor is enabled. > > > > > > RTL fix will be implemented CLKGATE_DIS_PSL_A bit 28-46520h. > > > > > > As per W/A 1604331009, Disable cursor clock gating in HDR mode. > > > > > > > > > > > > Bspec : 33451 > > > > > > > > > > > > Changes since V3: > > > > > > - Disable WA when not in HDR mode or cursor plane not > active - Ville > > > > > > - Extract required args from crtc_state - Ville > > > > > > - Create HDR mode API using bdw_set_pipemisc ref - Ville > > > > > > - Tested with HDR video as well full setmode, WA applies and > > > > > disables > > > > > > Changes since V2: > > > > > > - Made it general gen11 WA > > > > > > - Removed WA needed check > > > > > > - Added cursor plane active check > > > > > > - Once WA enable, software will not disable Changes since V1: > > > > > > - Modified way CLKGATE_DIS_PSL bit 28 was modified > > > > > > > > > > > > Cc: Souza Jose <jose.souza@intel.com> > > > > > > Signed-off-by: Tejas Upadhyay > > > > > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > > > > > --- > > > > > > drivers/gpu/drm/i915/display/intel_display.c | 27 > > > ++++++++++++++++++++ > > > > > > drivers/gpu/drm/i915/i915_reg.h | 5 ++++ > > > > > > 2 files changed, 32 insertions(+) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > > > > index 6be1b31af07b..e1ea03b918df 100644 > > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > > > @@ -358,6 +358,13 @@ static void intel_update_czclk(struct > > > > > drm_i915_private *dev_priv) > > > > > > dev_priv->czclk_freq); > > > > > > } > > > > > > > > > > > > +static bool > > > > > > +is_hdr_mode(const struct intel_crtc_state *crtc_state) { > > > > > > + return (crtc_state->active_planes & ~(icl_hdr_plane_mask() | > > > > > > + BIT(PLANE_CURSOR))) == 0; > > > > > > +} > > > > > > > > > > Please use this in bdw_set_pipemisc() as well. This could be a > > > > > separate prep patch actually. > > > > > > > > > > > + > > > > > > /* WA Display #0827: Gen9:all */ static void > > > > > > skl_wa_827(struct drm_i915_private *dev_priv, enum pipe pipe, > > > > > > bool > > > > > > enable) @@ -383,6 +390,23 @@ icl_wa_scalerclkgating(struct > > > > > drm_i915_private *dev_priv, enum pipe pipe, > > > > > > intel_de_read(dev_priv, > > > > > > CLKGATE_DIS_PSL(pipe)) & ~DPFR_GATING_DIS); } > > > > > > > > > > > > +/* Wa_1604331009:icl,jsl,ehl */ > > > > > > + static void > > > > > > +icl_wa_cursorclkgating(const struct intel_crtc_state *crtc_state) { > > > > > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > > > > > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > > > > > + > > > > > > + if (is_hdr_mode(crtc_state) && > > > > > > + crtc_state->active_planes & BIT(PLANE_CURSOR) && > > > > > > + IS_GEN(dev_priv, 11)) > > > > > > + intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc- > > >pipe), > > > > > > + CURSOR_GATING_DIS, > > CURSOR_GATING_DIS); > > > > > > + else > > > > > > + intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc- > > >pipe), > > > > > > + CURSOR_GATING_DIS, 0); } > > > > > > + > > > > > > static bool > > > > > > is_trans_port_sync_slave(const struct intel_crtc_state > > > > > > *crtc_state) { @@ -2939,6 +2963,9 @@ static void > > > > > > intel_pre_plane_update(struct intel_atomic_state *state, > > > > > > needs_scalerclk_wa(new_crtc_state)) > > > > > > icl_wa_scalerclkgating(dev_priv, pipe, true); > > > > > > > > > > > > + /* Wa_1604331009:icl,jsl,ehl */ > > > > > > + icl_wa_cursorclkgating(new_crtc_state); > > > > > > > > > > This looks a bit wrong. We shouldn't turn the clock gating back > > > > > on until after HDR mode has been disabled. > > > > > > > > > > So please model this after skl_wa_827() and > > > > > icl_wa_scalerclkgating() so that > > > > > a) the ordering is correct, and b) the code between all three > > > > > w/as looks consistent. > > > > > > > > I did not get what you are suggesting here. Can you please put psudo? > > > > Currently as far as I see icl_wa_cursorclkgating is already > > > > modelled after > > > skl_wa_827() and icl_wa_scalerclkgating(). Are referring same Or > > > something else? > > > > > > It should look something like: > > > > > > intel_pre_plane_update() > > > { > > > if (!needs_cursorclk_wa(old_crtc_state) && > > > needs_cursorclk_wa(new_crtc_state)) > > > icl_wa_cursorclkgating(..., true); } > > > > > > intel_post_plane_update() > > > { > > > if (needs_cursorclk_wa(old_crtc_state) && > > > !needs_cursorclk_wa(new_crtc_state)) > > > icl_wa_cursorclkgating(..., false); } > > > > > > > Tejas : In the previous version it was done this way only. But after > > review comments from Jose I had to change it. See "Changes since V2" > > section. Also you can check in previous versions of patch. > > > > > > > > -- > > > Ville Syrjälä > > > Intel
Hi Ville, I have posted single patch for HDR mode here https://patchwork.freedesktop.org/series/94428/#rev1 . Please review and ack. I will post " drm/i915/gen11: Disable cursor clock gating in HDR mode " on top of that patch. Thanks, Tejas > -----Original Message----- > From: Surendrakumar Upadhyay, TejaskumarX > Sent: 06 September 2021 11:41 > To: 'Ville Syrjälä' <ville.syrjala@linux.intel.com> > Cc: 'intel-gfx@lists.freedesktop.org' <intel-gfx@lists.freedesktop.org>; Souza, > Jose <jose.souza@intel.com>; Pandey, Hariom <hariom.pandey@intel.com> > Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock > gating in HDR mode > > Hi Ville/Jose, > > I hope you both discussed as in next version I will incorporate all changes as > per Ville's final suggestion. Please let me know if you guys think otherwise. > > Thanks, > Tejas > > > -----Original Message----- > > From: Surendrakumar Upadhyay, TejaskumarX > > Sent: 02 September 2021 18:37 > > To: 'Ville Syrjälä' <ville.syrjala@linux.intel.com> > > Cc: 'intel-gfx@lists.freedesktop.org' > > <intel-gfx@lists.freedesktop.org>; Souza, Jose <jose.souza@intel.com>; > > Pandey, Hariom <hariom.pandey@intel.com> > > Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor > > clock gating in HDR mode > > > > + Hariom > > > > > -----Original Message----- > > > From: Surendrakumar Upadhyay, TejaskumarX > > > Sent: 02 September 2021 18:34 > > > To: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Cc: intel-gfx@lists.freedesktop.org; Souza, Jose > > > <jose.souza@intel.com> > > > Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor > > > clock gating in HDR mode > > > > > > > > > > > > > -----Original Message----- > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Sent: 02 September 2021 18:29 > > > > To: Surendrakumar Upadhyay, TejaskumarX > > > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > > > Cc: intel-gfx@lists.freedesktop.org > > > > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor > > > > clock gating in HDR mode > > > > > > > > On Thu, Sep 02, 2021 at 11:07:06AM +0000, Surendrakumar Upadhyay, > > > > TejaskumarX wrote: > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > Sent: 01 September 2021 19:19 > > > > > > To: Surendrakumar Upadhyay, TejaskumarX > > > > > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > > > > > Cc: intel-gfx@lists.freedesktop.org > > > > > > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable > > > > > > cursor clock gating in HDR mode > > > > > > > > > > > > On Tue, Jun 22, 2021 at 03:04:24PM +0530, Tejas Upadhyay wrote: > > > > > > > Display underrun in HDR mode when cursor is enabled. > > > > > > > RTL fix will be implemented CLKGATE_DIS_PSL_A bit 28-46520h. > > > > > > > As per W/A 1604331009, Disable cursor clock gating in HDR mode. > > > > > > > > > > > > > > Bspec : 33451 > > > > > > > > > > > > > > Changes since V3: > > > > > > > - Disable WA when not in HDR mode or cursor plane not > > active - Ville > > > > > > > - Extract required args from crtc_state - Ville > > > > > > > - Create HDR mode API using bdw_set_pipemisc ref - Ville > > > > > > > - Tested with HDR video as well full setmode, WA applies > > > > > > > and > > > > > > disables > > > > > > > Changes since V2: > > > > > > > - Made it general gen11 WA > > > > > > > - Removed WA needed check > > > > > > > - Added cursor plane active check > > > > > > > - Once WA enable, software will not disable Changes since V1: > > > > > > > - Modified way CLKGATE_DIS_PSL bit 28 was modified > > > > > > > > > > > > > > Cc: Souza Jose <jose.souza@intel.com> > > > > > > > Signed-off-by: Tejas Upadhyay > > > > > > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > > > > > > --- > > > > > > > drivers/gpu/drm/i915/display/intel_display.c | 27 > > > > ++++++++++++++++++++ > > > > > > > drivers/gpu/drm/i915/i915_reg.h | 5 ++++ > > > > > > > 2 files changed, 32 insertions(+) > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > > > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > > > > > index 6be1b31af07b..e1ea03b918df 100644 > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > > > > @@ -358,6 +358,13 @@ static void intel_update_czclk(struct > > > > > > drm_i915_private *dev_priv) > > > > > > > dev_priv->czclk_freq); > > > > > > > } > > > > > > > > > > > > > > +static bool > > > > > > > +is_hdr_mode(const struct intel_crtc_state *crtc_state) { > > > > > > > + return (crtc_state->active_planes & ~(icl_hdr_plane_mask() | > > > > > > > + BIT(PLANE_CURSOR))) == 0; } > > > > > > > > > > > > Please use this in bdw_set_pipemisc() as well. This could be a > > > > > > separate prep patch actually. > > > > > > > > > > > > > + > > > > > > > /* WA Display #0827: Gen9:all */ static void > > > > > > > skl_wa_827(struct drm_i915_private *dev_priv, enum pipe > > > > > > > pipe, bool > > > > > > > enable) @@ -383,6 +390,23 @@ icl_wa_scalerclkgating(struct > > > > > > drm_i915_private *dev_priv, enum pipe pipe, > > > > > > > intel_de_read(dev_priv, > > > > > > > CLKGATE_DIS_PSL(pipe)) & ~DPFR_GATING_DIS); } > > > > > > > > > > > > > > +/* Wa_1604331009:icl,jsl,ehl */ > > > > > > > + static void > > > > > > > +icl_wa_cursorclkgating(const struct intel_crtc_state *crtc_state) { > > > > > > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > > > > > > > + struct drm_i915_private *dev_priv = > > > > > > > +to_i915(crtc->base.dev); > > > > > > > + > > > > > > > + if (is_hdr_mode(crtc_state) && > > > > > > > + crtc_state->active_planes & BIT(PLANE_CURSOR) && > > > > > > > + IS_GEN(dev_priv, 11)) > > > > > > > + intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc- > > > >pipe), > > > > > > > + CURSOR_GATING_DIS, > > > CURSOR_GATING_DIS); > > > > > > > + else > > > > > > > + intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc- > > > >pipe), > > > > > > > + CURSOR_GATING_DIS, 0); } > > > > > > > + > > > > > > > static bool > > > > > > > is_trans_port_sync_slave(const struct intel_crtc_state > > > > > > > *crtc_state) { @@ -2939,6 +2963,9 @@ static void > > > > > > > intel_pre_plane_update(struct intel_atomic_state *state, > > > > > > > needs_scalerclk_wa(new_crtc_state)) > > > > > > > icl_wa_scalerclkgating(dev_priv, pipe, true); > > > > > > > > > > > > > > + /* Wa_1604331009:icl,jsl,ehl */ > > > > > > > + icl_wa_cursorclkgating(new_crtc_state); > > > > > > > > > > > > This looks a bit wrong. We shouldn't turn the clock gating > > > > > > back on until after HDR mode has been disabled. > > > > > > > > > > > > So please model this after skl_wa_827() and > > > > > > icl_wa_scalerclkgating() so that > > > > > > a) the ordering is correct, and b) the code between all three > > > > > > w/as looks consistent. > > > > > > > > > > I did not get what you are suggesting here. Can you please put psudo? > > > > > Currently as far as I see icl_wa_cursorclkgating is already > > > > > modelled after > > > > skl_wa_827() and icl_wa_scalerclkgating(). Are referring same Or > > > > something else? > > > > > > > > It should look something like: > > > > > > > > intel_pre_plane_update() > > > > { > > > > if (!needs_cursorclk_wa(old_crtc_state) && > > > > needs_cursorclk_wa(new_crtc_state)) > > > > icl_wa_cursorclkgating(..., true); } > > > > > > > > intel_post_plane_update() > > > > { > > > > if (needs_cursorclk_wa(old_crtc_state) && > > > > !needs_cursorclk_wa(new_crtc_state)) > > > > icl_wa_cursorclkgating(..., false); } > > > > > > > > > > Tejas : In the previous version it was done this way only. But after > > > review comments from Jose I had to change it. See "Changes since V2" > > > section. Also you can check in previous versions of patch. > > > > > > > > > > > -- > > > > Ville Syrjälä > > > > Intel
Hi Ville, Can you please help with reviewing below patch? Thanks, Tejas > -----Original Message----- > From: Surendrakumar Upadhyay, TejaskumarX > Sent: 07 September 2021 21:42 > To: 'Ville Syrjälä' <ville.syrjala@linux.intel.com> > Cc: 'intel-gfx@lists.freedesktop.org' <intel-gfx@lists.freedesktop.org>; Souza, > Jose <jose.souza@intel.com>; Pandey, Hariom <hariom.pandey@intel.com> > Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock > gating in HDR mode > > Hi Ville, > > I have posted single patch for HDR mode here > https://patchwork.freedesktop.org/series/94428/#rev1 . Please review and > ack. I will post " drm/i915/gen11: Disable cursor clock gating in HDR mode " > on top of that patch. > > Thanks, > Tejas > > > -----Original Message----- > > From: Surendrakumar Upadhyay, TejaskumarX > > Sent: 06 September 2021 11:41 > > To: 'Ville Syrjälä' <ville.syrjala@linux.intel.com> > > Cc: 'intel-gfx@lists.freedesktop.org' > > <intel-gfx@lists.freedesktop.org>; Souza, Jose <jose.souza@intel.com>; > > Pandey, Hariom <hariom.pandey@intel.com> > > Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor > > clock gating in HDR mode > > > > Hi Ville/Jose, > > > > I hope you both discussed as in next version I will incorporate all > > changes as per Ville's final suggestion. Please let me know if you guys think > otherwise. > > > > Thanks, > > Tejas > > > > > -----Original Message----- > > > From: Surendrakumar Upadhyay, TejaskumarX > > > Sent: 02 September 2021 18:37 > > > To: 'Ville Syrjälä' <ville.syrjala@linux.intel.com> > > > Cc: 'intel-gfx@lists.freedesktop.org' > > > <intel-gfx@lists.freedesktop.org>; Souza, Jose > > > <jose.souza@intel.com>; Pandey, Hariom <hariom.pandey@intel.com> > > > Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor > > > clock gating in HDR mode > > > > > > + Hariom > > > > > > > -----Original Message----- > > > > From: Surendrakumar Upadhyay, TejaskumarX > > > > Sent: 02 September 2021 18:34 > > > > To: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Cc: intel-gfx@lists.freedesktop.org; Souza, Jose > > > > <jose.souza@intel.com> > > > > Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor > > > > clock gating in HDR mode > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > Sent: 02 September 2021 18:29 > > > > > To: Surendrakumar Upadhyay, TejaskumarX > > > > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > > > > Cc: intel-gfx@lists.freedesktop.org > > > > > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable > > > > > cursor clock gating in HDR mode > > > > > > > > > > On Thu, Sep 02, 2021 at 11:07:06AM +0000, Surendrakumar > > > > > Upadhyay, TejaskumarX wrote: > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > Sent: 01 September 2021 19:19 > > > > > > > To: Surendrakumar Upadhyay, TejaskumarX > > > > > > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > > > > > > Cc: intel-gfx@lists.freedesktop.org > > > > > > > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable > > > > > > > cursor clock gating in HDR mode > > > > > > > > > > > > > > On Tue, Jun 22, 2021 at 03:04:24PM +0530, Tejas Upadhyay wrote: > > > > > > > > Display underrun in HDR mode when cursor is enabled. > > > > > > > > RTL fix will be implemented CLKGATE_DIS_PSL_A bit 28-46520h. > > > > > > > > As per W/A 1604331009, Disable cursor clock gating in HDR > mode. > > > > > > > > > > > > > > > > Bspec : 33451 > > > > > > > > > > > > > > > > Changes since V3: > > > > > > > > - Disable WA when not in HDR mode or cursor plane not > > > active - Ville > > > > > > > > - Extract required args from crtc_state - Ville > > > > > > > > - Create HDR mode API using bdw_set_pipemisc ref - Ville > > > > > > > > - Tested with HDR video as well full setmode, WA applies > > > > > > > > and > > > > > > > disables > > > > > > > > Changes since V2: > > > > > > > > - Made it general gen11 WA > > > > > > > > - Removed WA needed check > > > > > > > > - Added cursor plane active check > > > > > > > > - Once WA enable, software will not disable Changes since > V1: > > > > > > > > - Modified way CLKGATE_DIS_PSL bit 28 was modified > > > > > > > > > > > > > > > > Cc: Souza Jose <jose.souza@intel.com> > > > > > > > > Signed-off-by: Tejas Upadhyay > > > > > > > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > > > > > > > --- > > > > > > > > drivers/gpu/drm/i915/display/intel_display.c | 27 > > > > > ++++++++++++++++++++ > > > > > > > > drivers/gpu/drm/i915/i915_reg.h | 5 ++++ > > > > > > > > 2 files changed, 32 insertions(+) > > > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > > > > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > > > > > > index 6be1b31af07b..e1ea03b918df 100644 > > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > > > > > @@ -358,6 +358,13 @@ static void intel_update_czclk(struct > > > > > > > drm_i915_private *dev_priv) > > > > > > > > dev_priv->czclk_freq); > > > > > > > > } > > > > > > > > > > > > > > > > +static bool > > > > > > > > +is_hdr_mode(const struct intel_crtc_state *crtc_state) { > > > > > > > > + return (crtc_state->active_planes & ~(icl_hdr_plane_mask() | > > > > > > > > + BIT(PLANE_CURSOR))) == 0; } > > > > > > > > > > > > > > Please use this in bdw_set_pipemisc() as well. This could be > > > > > > > a separate prep patch actually. > > > > > > > > > > > > > > > + > > > > > > > > /* WA Display #0827: Gen9:all */ static void > > > > > > > > skl_wa_827(struct drm_i915_private *dev_priv, enum pipe > > > > > > > > pipe, bool > > > > > > > > enable) @@ -383,6 +390,23 @@ icl_wa_scalerclkgating(struct > > > > > > > drm_i915_private *dev_priv, enum pipe pipe, > > > > > > > > intel_de_read(dev_priv, > > > > > > > > CLKGATE_DIS_PSL(pipe)) & ~DPFR_GATING_DIS); } > > > > > > > > > > > > > > > > +/* Wa_1604331009:icl,jsl,ehl */ > > > > > > > > + static void > > > > > > > > +icl_wa_cursorclkgating(const struct intel_crtc_state *crtc_state) > { > > > > > > > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > > > > > > > > + struct drm_i915_private *dev_priv = > > > > > > > > +to_i915(crtc->base.dev); > > > > > > > > + > > > > > > > > + if (is_hdr_mode(crtc_state) && > > > > > > > > + crtc_state->active_planes & BIT(PLANE_CURSOR) && > > > > > > > > + IS_GEN(dev_priv, 11)) > > > > > > > > + intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc- > > > > >pipe), > > > > > > > > + CURSOR_GATING_DIS, > > > > CURSOR_GATING_DIS); > > > > > > > > + else > > > > > > > > + intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc- > > > > >pipe), > > > > > > > > + CURSOR_GATING_DIS, 0); } > > > > > > > > + > > > > > > > > static bool > > > > > > > > is_trans_port_sync_slave(const struct intel_crtc_state > > > > > > > > *crtc_state) { @@ -2939,6 +2963,9 @@ static void > > > > > > > > intel_pre_plane_update(struct intel_atomic_state *state, > > > > > > > > needs_scalerclk_wa(new_crtc_state)) > > > > > > > > icl_wa_scalerclkgating(dev_priv, pipe, true); > > > > > > > > > > > > > > > > + /* Wa_1604331009:icl,jsl,ehl */ > > > > > > > > + icl_wa_cursorclkgating(new_crtc_state); > > > > > > > > > > > > > > This looks a bit wrong. We shouldn't turn the clock gating > > > > > > > back on until after HDR mode has been disabled. > > > > > > > > > > > > > > So please model this after skl_wa_827() and > > > > > > > icl_wa_scalerclkgating() so that > > > > > > > a) the ordering is correct, and b) the code between all > > > > > > > three w/as looks consistent. > > > > > > > > > > > > I did not get what you are suggesting here. Can you please put > psudo? > > > > > > Currently as far as I see icl_wa_cursorclkgating is already > > > > > > modelled after > > > > > skl_wa_827() and icl_wa_scalerclkgating(). Are referring same Or > > > > > something else? > > > > > > > > > > It should look something like: > > > > > > > > > > intel_pre_plane_update() > > > > > { > > > > > if (!needs_cursorclk_wa(old_crtc_state) && > > > > > needs_cursorclk_wa(new_crtc_state)) > > > > > icl_wa_cursorclkgating(..., true); } > > > > > > > > > > intel_post_plane_update() > > > > > { > > > > > if (needs_cursorclk_wa(old_crtc_state) && > > > > > !needs_cursorclk_wa(new_crtc_state)) > > > > > icl_wa_cursorclkgating(..., false); } > > > > > > > > > > > > > Tejas : In the previous version it was done this way only. But > > > > after review comments from Jose I had to change it. See "Changes since > V2" > > > > section. Also you can check in previous versions of patch. > > > > > > > > > > > > > > -- > > > > > Ville Syrjälä > > > > > Intel
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 6be1b31af07b..e1ea03b918df 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -358,6 +358,13 @@ static void intel_update_czclk(struct drm_i915_private *dev_priv) dev_priv->czclk_freq); } +static bool +is_hdr_mode(const struct intel_crtc_state *crtc_state) +{ + return (crtc_state->active_planes & ~(icl_hdr_plane_mask() | + BIT(PLANE_CURSOR))) == 0; +} + /* WA Display #0827: Gen9:all */ static void skl_wa_827(struct drm_i915_private *dev_priv, enum pipe pipe, bool enable) @@ -383,6 +390,23 @@ icl_wa_scalerclkgating(struct drm_i915_private *dev_priv, enum pipe pipe, intel_de_read(dev_priv, CLKGATE_DIS_PSL(pipe)) & ~DPFR_GATING_DIS); } +/* Wa_1604331009:icl,jsl,ehl */ + static void +icl_wa_cursorclkgating(const struct intel_crtc_state *crtc_state) +{ + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + + if (is_hdr_mode(crtc_state) && + crtc_state->active_planes & BIT(PLANE_CURSOR) && + IS_GEN(dev_priv, 11)) + intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe), + CURSOR_GATING_DIS, CURSOR_GATING_DIS); + else + intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe), + CURSOR_GATING_DIS, 0); +} + static bool is_trans_port_sync_slave(const struct intel_crtc_state *crtc_state) { @@ -2939,6 +2963,9 @@ static void intel_pre_plane_update(struct intel_atomic_state *state, needs_scalerclk_wa(new_crtc_state)) icl_wa_scalerclkgating(dev_priv, pipe, true); + /* Wa_1604331009:icl,jsl,ehl */ + icl_wa_cursorclkgating(new_crtc_state); + /* * Vblank time updates from the shadow to live plane control register * are blocked if the memory self-refresh mode is active at that diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index c857fafb8a30..703d708c773e 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4235,6 +4235,11 @@ enum { #define INF_UNIT_LEVEL_CLKGATE _MMIO(0x9560) #define CGPSF_CLKGATE_DIS (1 << 3) +/* + * GEN11 clock gating regs + */ +#define CURSOR_GATING_DIS BIT(28) + /* * Display engine regs */
Display underrun in HDR mode when cursor is enabled. RTL fix will be implemented CLKGATE_DIS_PSL_A bit 28-46520h. As per W/A 1604331009, Disable cursor clock gating in HDR mode. Bspec : 33451 Changes since V3: - Disable WA when not in HDR mode or cursor plane not active - Ville - Extract required args from crtc_state - Ville - Create HDR mode API using bdw_set_pipemisc ref - Ville - Tested with HDR video as well full setmode, WA applies and disables Changes since V2: - Made it general gen11 WA - Removed WA needed check - Added cursor plane active check - Once WA enable, software will not disable Changes since V1: - Modified way CLKGATE_DIS_PSL bit 28 was modified Cc: Souza Jose <jose.souza@intel.com> Signed-off-by: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 27 ++++++++++++++++++++ drivers/gpu/drm/i915/i915_reg.h | 5 ++++ 2 files changed, 32 insertions(+)