On Thu, Jun 03, 2021 at 08:37:22AM -0700, Kulkarni, Vandita wrote: > > -----Original Message----- > > From: Manna, Animesh <animesh.manna@xxxxxxxxx> > > Sent: Thursday, June 3, 2021 7:24 PM > > To: Kulkarni, Vandita <vandita.kulkarni@xxxxxxxxx>; Nikula, Jani > > <jani.nikula@xxxxxxxxx>; Saarinen, Jani <jani.saarinen@xxxxxxxxx>; intel- > > gfx@xxxxxxxxxxxxxxxxxxxxx > > Cc: Navare, Manasi D <manasi.d.navare@xxxxxxxxx> > > Subject: RE: [PATCH] drm/i915/dsc: Remove redundant checks in > > DSC disable > > > > > > > > > -----Original Message----- > > > From: Kulkarni, Vandita <vandita.kulkarni@xxxxxxxxx> > > > Sent: Thursday, June 3, 2021 4:55 PM > > > To: Nikula, Jani <jani.nikula@xxxxxxxxx>; Saarinen, Jani > > > <jani.saarinen@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > Cc: Manna, Animesh <animesh.manna@xxxxxxxxx>; Navare, Manasi D > > > <manasi.d.navare@xxxxxxxxx> > > > Subject: RE: [PATCH] drm/i915/dsc: Remove redundant checks > > > in DSC disable > > > > > > > -----Original Message----- > > > > From: Nikula, Jani <jani.nikula@xxxxxxxxx> > > > > Sent: Thursday, June 3, 2021 3:11 PM > > > > To: Kulkarni, Vandita <vandita.kulkarni@xxxxxxxxx>; Saarinen, Jani > > > > <jani.saarinen@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > > Cc: Manna, Animesh <animesh.manna@xxxxxxxxx>; Navare, Manasi D > > > > <manasi.d.navare@xxxxxxxxx> > > > > Subject: RE: [PATCH] drm/i915/dsc: Remove redundant > > > > checks in DSC disable > > > > > > > > On Thu, 03 Jun 2021, "Kulkarni, Vandita" > > > > <vandita.kulkarni@xxxxxxxxx> > > > > wrote: > > > > >> -----Original Message----- > > > > >> From: Saarinen, Jani <jani.saarinen@xxxxxxxxx> > > > > >> Sent: Thursday, June 3, 2021 1:07 PM > > > > >> To: Kulkarni, Vandita <vandita.kulkarni@xxxxxxxxx>; intel- > > > > >> gfx@xxxxxxxxxxxxxxxxxxxxx > > > > >> Cc: Nikula, Jani <jani.nikula@xxxxxxxxx> > > > > >> Subject: RE: [PATCH] drm/i915/dsc: Remove redundant > > > > >> checks in DSC disable > > > > >> > > > > >> Hi, > > > > >> > -----Original Message----- > > > > >> > From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On > > > > >> > Behalf Of Vandita Kulkarni > > > > >> > Sent: torstai 3. kesäkuuta 2021 9.54 > > > > >> > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > > >> > Cc: Nikula, Jani <jani.nikula@xxxxxxxxx> > > > > >> > Subject: [PATCH] drm/i915/dsc: Remove redundant > > > > >> > checks in DSC disable > > > > >> > > > > > >> > There can be a chance that pre os has enabled DSC and driver's > > > > >> > compute config would not need dsc to be enabled, in such case > > > > >> > if we check on compute config's compression state to disable, > > > > >> > we might end up in state > > > > >> mismatch. > > > > >> > > > > >> I assume this fixes real gitlab issue too? > > > > > Okay, will add the tag > > > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3537 > > > > > > > > See https://lore.kernel.org/r/87fsxzp9qx.fsf@xxxxxxxxx > > > > > > > > The problem is with ->bigjoiner, not the entire statement. > > > Thanks for pointing this out, true that bigjoiner not being enabled > > > will stop dsc disabling. > > > The bigjoiner check was making the entire condition check unnecessary. > > > > > > Will update and refloat. > > > > Hi Jani/Vandita, > > > > For uncompressed bigjoiner case if we want to use the same function to > > clear the dsc_ctrl1 register we may need to remove both the condition > > check. > > As for uncompressed bigjoiner case, compression_enable Will be 0 and will > > block in clearing the dss_ctl1_reg. > > Yes, I was going through and found that bit 20 and 21 of dss_ctl1 are being used > for uncompressed joiner. > So when dsc is not enabled to avoid writing the register we could add > below code . > > if (dsc) > clear dss_ctl2 > if ( bigjoiner | dsc) > clear dss_ctl1; > return; > > bigjoiner = 1 and dsc = 0 - uncompressed , I think there is no harm in clearing dsc bits again > bigjoiner = 1 and dsc = 1 - compressed - uncompressed bits are already 0 > bigjoiner = 0 and dsc= 1 - just dsc - clear dsc rest are 0s already > bigjoiner = 0 and dsc = 0 do nothing, return > > If I have missed any corner case, please let me know. > > Thanks, > Vandita I think in the original code the condition was just reversed, instead it should be : if !(dsc_en || bigjoiner_en) { write 0 to dss ctl 1 write 0 to dss ctl 2 } So here basically it meets all the conditions you mentioned Vandita: - only when both dsc and bigjoiner are 0, it will do nothing - In all other cases DSC + Bigjoiner : Clear all bits including uncompressed bits which shd be 0 already - In dsc = 0, bigjoiner = 1 (uncompressed), it will clear both again which is okay since dsc bits are already 0 Does this make sense? Regards Manasi > > > > Regards, > > Animesh > > > > > > Thanks, > > > Vandita > > > > > > > > > > > > BR, > > > > Jani. > > > > > > > > > > > > > > Thanks, > > > > > Vandita > > > > >> > > > > >> > > > > > >> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@xxxxxxxxx> > > > > >> > --- > > > > >> > drivers/gpu/drm/i915/display/intel_vdsc.c | 4 ---- > > > > >> > 1 file changed, 4 deletions(-) > > > > >> > > > > > >> > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c > > > > >> > b/drivers/gpu/drm/i915/display/intel_vdsc.c > > > > >> > index 19cd9531c115..b05a96011d93 100644 > > > > >> > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c > > > > >> > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c > > > > >> > @@ -1161,10 +1161,6 @@ void intel_dsc_disable(const struct > > > > >> > intel_crtc_state > > > > >> > *old_crtc_state) > > > > >> > struct intel_crtc *crtc = to_intel_crtc(old_crtc_state- > > >uapi.crtc); > > > > >> > struct drm_i915_private *dev_priv = to_i915(crtc- > > >base.dev); > > > > >> > > > > > >> > - if (!(old_crtc_state->dsc.compression_enable && > > > > >> > - old_crtc_state->bigjoiner)) > > > > >> > - return; > > > > >> > - > > > > >> > intel_de_write(dev_priv, dss_ctl1_reg(old_crtc_state), 0); > > > > >> > intel_de_write(dev_priv, dss_ctl2_reg(old_crtc_state), 0); } > > > > >> > -- > > > > >> > 2.21.0.5.gaeb582a > > > > >> > > > > > >> > _______________________________________________ > > > > >> > Intel-gfx mailing list > > > > >> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > > >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > > > -- > > > > Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
