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@xxxxxxxxxxxxxxx> > Cc: 'intel-gfx@xxxxxxxxxxxxxxxxxxxxx' <intel-gfx@xxxxxxxxxxxxxxxxxxxxx>; Souza, > Jose <jose.souza@xxxxxxxxx>; Pandey, Hariom <hariom.pandey@xxxxxxxxx> > Subject: RE: [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@xxxxxxxxxxxxxxx> > > Cc: 'intel-gfx@xxxxxxxxxxxxxxxxxxxxx' > > <intel-gfx@xxxxxxxxxxxxxxxxxxxxx>; Souza, Jose <jose.souza@xxxxxxxxx>; > > Pandey, Hariom <hariom.pandey@xxxxxxxxx> > > Subject: RE: [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@xxxxxxxxxxxxxxx> > > > Cc: 'intel-gfx@xxxxxxxxxxxxxxxxxxxxx' > > > <intel-gfx@xxxxxxxxxxxxxxxxxxxxx>; Souza, Jose > > > <jose.souza@xxxxxxxxx>; Pandey, Hariom <hariom.pandey@xxxxxxxxx> > > > Subject: RE: [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@xxxxxxxxxxxxxxx> > > > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Souza, Jose > > > > <jose.souza@xxxxxxxxx> > > > > Subject: RE: [PATCH V4] drm/i915/gen11: Disable cursor > > > > clock gating in HDR mode > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > > Sent: 02 September 2021 18:29 > > > > > To: Surendrakumar Upadhyay, TejaskumarX > > > > > <tejaskumarx.surendrakumar.upadhyay@xxxxxxxxx> > > > > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > > > Subject: Re: [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@xxxxxxxxxxxxxxx> > > > > > > > Sent: 01 September 2021 19:19 > > > > > > > To: Surendrakumar Upadhyay, TejaskumarX > > > > > > > <tejaskumarx.surendrakumar.upadhyay@xxxxxxxxx> > > > > > > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > > > > > Subject: Re: [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@xxxxxxxxx> > > > > > > > > Signed-off-by: Tejas Upadhyay > > > > > > > > <tejaskumarx.surendrakumar.upadhyay@xxxxxxxxx> > > > > > > > > --- > > > > > > > > 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
