> -----Original Message----- > From: Kandpal, Suraj <suraj.kandpal@xxxxxxxxx> > Sent: Wednesday, September 11, 2024 4:10 PM > To: Murthy, Arun R <arun.r.murthy@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Murthy, Arun R <arun.r.murthy@xxxxxxxxx> > Subject: RE: [PATCHv2 4/5] drm/i915/histogram: histogram delay counter > doesnt reset > > > > > -----Original Message----- > > From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of > > Arun R Murthy > > Sent: Wednesday, August 21, 2024 3:54 PM > > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > Cc: Murthy, Arun R <arun.r.murthy@xxxxxxxxx> > > Subject: [PATCHv2 4/5] drm/i915/histogram: histogram delay counter > > doesnt reset > > > > The delay counter for histogram does not reset and as a result the > > histogram bin never gets updated. Woraround would be to use save and > > restore histogram > > Typo "Workaround" > Done > > register. > > > > Signed-off-by: Arun R Murthy <arun.r.murthy@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/display/intel_histogram.c | 17 +++++++++++++++++ > > drivers/gpu/drm/i915/display/intel_histogram.h | 1 + > > 2 files changed, 18 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_histogram.c > > b/drivers/gpu/drm/i915/display/intel_histogram.c > > index ad4f75607ccb..189f7ccd6df8 100644 > > --- a/drivers/gpu/drm/i915/display/intel_histogram.c > > +++ b/drivers/gpu/drm/i915/display/intel_histogram.c > > @@ -36,6 +36,11 @@ static void intel_histogram_handle_int_work(struct > > work_struct *work) > > u32 dpstbin; > > int i, try = 0; > > > > + /* Wa: 14014889975 */ > > + if (IS_DISPLAY_VER(i915, 12, 13)) > > + intel_de_rmw(i915, DPST_CTL(intel_crtc->pipe), > > + DPST_CTL_RESTORE, 0); > > + > > /* > > * TODO: PSR to be exited while reading the Histogram data > > * Set DPST_CTL Bin Reg function select to TC @@ -77,6 +82,12 @@ > > static void intel_histogram_handle_int_work(struct work_struct *work) > > histogram_event)) > > drm_err(&i915->drm, "sending HISTOGRAM event failed\n"); > > > > + /* Wa: 14014889975 */ > > + if (IS_DISPLAY_VER(i915, 12, 13)) > > + /* Write the value read from DPST_CTL to DPST_CTL.Interrupt > > Delay Counter(bit 23:16) */ > > + intel_de_write(i915, DPST_CTL(intel_crtc->pipe), > > intel_de_read(i915, > > + DPST_CTL(intel_crtc->pipe)) | > DPST_CTL_RESTORE); > > + > > /* Enable histogram interrupt */ > > intel_de_rmw(i915, DPST_GUARD(intel_crtc->pipe), > > DPST_GUARD_HIST_INT_EN, > > DPST_GUARD_HIST_INT_EN); > > @@ -140,6 +151,12 @@ static int intel_histogram_enable(struct > > intel_crtc > > *intel_crtc) > > /* Pipe Dithering should be enabled with GLOBAL_HIST */ > > intel_histogram_enable_dithering(i915, pipe); > > > > + /* Wa: 14014889975 */ > > + if (IS_DISPLAY_VER(i915, 12, 13)) > > + /* Write the value read from DPST_CTL to DPST_CTL.Interrupt > > Delay Counter(bit 23:16) */ > > + intel_de_write(i915, DPST_CTL(intel_crtc->pipe), > > intel_de_read(i915, > > + DPST_CTL(intel_crtc->pipe)) | > DPST_CTL_RESTORE); > > + > > Maybe put all the code in a function with required variables and one extra bool > variable that determines If it is being called when interrupt is being serviced or > when histogram is being enabled > Have a function to just execute one write function would be much costlier. > wa_14014889975(.., bool enable) > if (!IS_DISPLAY_VER(i915, 12, 13)) /* I think there is a function that > checks if display_ver is not it range to avoid the ! > return > if (enable) > /* Write the value read from DPST_CTL to DPST_CTL.Interrupt > Delay Counter(bit 23:16) */ > intel_de_write(i915, DPST_CTL(intel_crtc->pipe), > intel_de_read(i915, > DPST_CTL(intel_crtc->pipe)) | > DPST_CTL_RESTORE); > Else > intel_de_rmw(i915, DPST_CTL(intel_crtc->pipe), > DPST_CTL_RESTORE, 0); > > Also use intel_display instead of i915 > Done Thanks and Regards, Arun R Murthy --------------------