Message ID | 20241115160116.1436521-1-suraj.kandpal@intel.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | [1/6] drm/i915/wm: Initialize max_latency variable to appropriate value | expand |
> -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Suraj > Kandpal > Sent: 15 November 2024 21:31 > To: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > Cc: Govindapillai, Vinod <vinod.govindapillai@intel.com>; Nikula, Jani > <jani.nikula@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com> > Subject: [PATCH 1/6] drm/i915/wm: Initialize max_latency variable to > appropriate value > > Initialize max_latency variable to LNL_PKG_C_LATENCY_MASK which helps to > eliminate the else block and make the whole code a lot cleaner. > While we are at it group the initialized variable together. I suggest not to combine your functional change with other refactoring. Rest other suggested changes from previous revision looks good. > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > --- > drivers/gpu/drm/i915/display/skl_watermark.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c > b/drivers/gpu/drm/i915/display/skl_watermark.c > index 1a4c1fa24820..a49e8915346e 100644 > --- a/drivers/gpu/drm/i915/display/skl_watermark.c > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c > @@ -2857,9 +2857,8 @@ static int skl_wm_add_affected_planes(struct > intel_atomic_state *state, static void skl_program_dpkgc_latency(struct > drm_i915_private *i915, bool enable_dpkgc) { > - u32 max_latency = 0; > + u32 max_latency = LNL_PKG_C_LATENCY_MASK, added_wake_time = > 0; > u32 clear = 0, val = 0; > - u32 added_wake_time = 0; > > if (DISPLAY_VER(i915) < 20) > return; > @@ -2870,9 +2869,6 @@ skl_program_dpkgc_latency(struct > drm_i915_private *i915, bool enable_dpkgc) > max_latency = LNL_PKG_C_LATENCY_MASK; > added_wake_time = DSB_EXE_TIME + > i915->display.sagv.block_time_us; > - } else { > - max_latency = LNL_PKG_C_LATENCY_MASK; > - added_wake_time = 0; > } > > clear |= LNL_ADDED_WAKE_TIME_MASK | > LNL_PKG_C_LATENCY_MASK; > -- > 2.34.1
> -----Original Message----- > From: Golani, Mitulkumar Ajitkumar > <mitulkumar.ajitkumar.golani@intel.com> > Sent: Monday, December 2, 2024 2:46 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel-xe@lists.freedesktop.org; > intel-gfx@lists.freedesktop.org > Cc: Govindapillai, Vinod <vinod.govindapillai@intel.com>; Nikula, Jani > <jani.nikula@intel.com> > Subject: RE: [PATCH 1/6] drm/i915/wm: Initialize max_latency variable to > appropriate value > > > > > -----Original Message----- > > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of > > Suraj Kandpal > > Sent: 15 November 2024 21:31 > > To: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > > Cc: Govindapillai, Vinod <vinod.govindapillai@intel.com>; Nikula, Jani > > <jani.nikula@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com> > > Subject: [PATCH 1/6] drm/i915/wm: Initialize max_latency variable to > > appropriate value > > > > Initialize max_latency variable to LNL_PKG_C_LATENCY_MASK which helps > > to eliminate the else block and make the whole code a lot cleaner. > > While we are at it group the initialized variable together. > > I suggest not to combine your functional change with other refactoring. Rest > other suggested changes from previous revision looks good. > Sure will wait for your other reviews and rb's and then float latest revision Regards, Suraj Kandpal > > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > --- > > drivers/gpu/drm/i915/display/skl_watermark.c | 6 +----- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c > > b/drivers/gpu/drm/i915/display/skl_watermark.c > > index 1a4c1fa24820..a49e8915346e 100644 > > --- a/drivers/gpu/drm/i915/display/skl_watermark.c > > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c > > @@ -2857,9 +2857,8 @@ static int skl_wm_add_affected_planes(struct > > intel_atomic_state *state, static void > > skl_program_dpkgc_latency(struct drm_i915_private *i915, bool > enable_dpkgc) { > > - u32 max_latency = 0; > > + u32 max_latency = LNL_PKG_C_LATENCY_MASK, added_wake_time = > > 0; > > u32 clear = 0, val = 0; > > - u32 added_wake_time = 0; > > > > if (DISPLAY_VER(i915) < 20) > > return; > > @@ -2870,9 +2869,6 @@ skl_program_dpkgc_latency(struct > > drm_i915_private *i915, bool enable_dpkgc) > > max_latency = LNL_PKG_C_LATENCY_MASK; > > added_wake_time = DSB_EXE_TIME + > > i915->display.sagv.block_time_us; > > - } else { > > - max_latency = LNL_PKG_C_LATENCY_MASK; > > - added_wake_time = 0; > > } > > > > clear |= LNL_ADDED_WAKE_TIME_MASK | > > LNL_PKG_C_LATENCY_MASK; > > -- > > 2.34.1
On Fri, 15 Nov 2024, Suraj Kandpal <suraj.kandpal@intel.com> wrote: > Initialize max_latency variable to LNL_PKG_C_LATENCY_MASK which helps > to eliminate the else block and make the whole code a lot cleaner. > While we are at it group the initialized variable together. > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > --- > drivers/gpu/drm/i915/display/skl_watermark.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c > index 1a4c1fa24820..a49e8915346e 100644 > --- a/drivers/gpu/drm/i915/display/skl_watermark.c > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c > @@ -2857,9 +2857,8 @@ static int skl_wm_add_affected_planes(struct intel_atomic_state *state, > static void > skl_program_dpkgc_latency(struct drm_i915_private *i915, bool enable_dpkgc) > { > - u32 max_latency = 0; > + u32 max_latency = LNL_PKG_C_LATENCY_MASK, added_wake_time = 0; Whoops. Please read your own commits through before sending. BR, Jani. > u32 clear = 0, val = 0; > - u32 added_wake_time = 0; > > if (DISPLAY_VER(i915) < 20) > return; > @@ -2870,9 +2869,6 @@ skl_program_dpkgc_latency(struct drm_i915_private *i915, bool enable_dpkgc) > max_latency = LNL_PKG_C_LATENCY_MASK; > added_wake_time = DSB_EXE_TIME + > i915->display.sagv.block_time_us; > - } else { > - max_latency = LNL_PKG_C_LATENCY_MASK; > - added_wake_time = 0; > } > > clear |= LNL_ADDED_WAKE_TIME_MASK | LNL_PKG_C_LATENCY_MASK;
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index 1a4c1fa24820..a49e8915346e 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -2857,9 +2857,8 @@ static int skl_wm_add_affected_planes(struct intel_atomic_state *state, static void skl_program_dpkgc_latency(struct drm_i915_private *i915, bool enable_dpkgc) { - u32 max_latency = 0; + u32 max_latency = LNL_PKG_C_LATENCY_MASK, added_wake_time = 0; u32 clear = 0, val = 0; - u32 added_wake_time = 0; if (DISPLAY_VER(i915) < 20) return; @@ -2870,9 +2869,6 @@ skl_program_dpkgc_latency(struct drm_i915_private *i915, bool enable_dpkgc) max_latency = LNL_PKG_C_LATENCY_MASK; added_wake_time = DSB_EXE_TIME + i915->display.sagv.block_time_us; - } else { - max_latency = LNL_PKG_C_LATENCY_MASK; - added_wake_time = 0; } clear |= LNL_ADDED_WAKE_TIME_MASK | LNL_PKG_C_LATENCY_MASK;
Initialize max_latency variable to LNL_PKG_C_LATENCY_MASK which helps to eliminate the else block and make the whole code a lot cleaner. While we are at it group the initialized variable together. Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> --- drivers/gpu/drm/i915/display/skl_watermark.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)