| Message ID | 20240711044905.3306882-1-suraj.kandpal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/dpkgc: Add VRR condition for DPKGC Enablement | expand |
> -----Original Message----- > From: Kandpal, Suraj <suraj.kandpal@intel.com> > Sent: Thursday, July 11, 2024 10:19 AM > To: intel-gfx@lists.freedesktop.org > Cc: Manna, Animesh <animesh.manna@intel.com>; B, Jeevan > <jeevan.b@intel.com>; Golani, Mitulkumar Ajitkumar > <mitulkumar.ajitkumar.golani@intel.com>; Naladala, Ramanaidu > <ramanaidu.naladala@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com> > Subject: [PATCH] drm/i915/dpkgc: Add VRR condition for DPKGC Enablement > > DPKGC can now be enabled with VRR enabled if Vmin = Vmax = Flipline is met. > > Bspec: 68986 > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > --- > drivers/gpu/drm/i915/display/skl_watermark.c | 24 +++++++++++--------- > 1 file changed, 13 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c > b/drivers/gpu/drm/i915/display/skl_watermark.c > index a2726364b34d..045c7cac166b 100644 > --- a/drivers/gpu/drm/i915/display/skl_watermark.c > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c > @@ -2830,17 +2830,17 @@ static int skl_wm_add_affected_planes(struct > intel_atomic_state *state, } > > /* > - * If Fixed Refresh Rate: > + * If Fixed Refresh Rate or For VRR case Vmin = Vmax = Flipline: > * Program DEEP PKG_C_LATENCY Pkg C with highest valid latency from > * watermark level1 and up and above. If watermark level 1 is > * invalid program it with all 1's. > * Program PKG_C_LATENCY Added Wake Time = DSB execution time > - * If Variable Refresh Rate: > + * If Variable Refresh Rate where Vmin != Vmax != Flipline: > * Program DEEP PKG_C_LATENCY Pkg C with all 1's. > * Program PKG_C_LATENCY Added Wake Time = 0 > */ > static void > -skl_program_dpkgc_latency(struct drm_i915_private *i915, bool vrr_enabled) > +skl_program_dpkgc_latency(struct drm_i915_private *i915, bool > +enable_dpkgc) > { > u32 max_latency = 0; > u32 clear = 0, val = 0; > @@ -2849,15 +2849,15 @@ skl_program_dpkgc_latency(struct > drm_i915_private *i915, bool vrr_enabled) > if (DISPLAY_VER(i915) < 20) > return; > > - if (vrr_enabled) { > - max_latency = LNL_PKG_C_LATENCY_MASK; > - added_wake_time = 0; > - } else { > + if (enable_dpkgc) { > max_latency = skl_watermark_max_latency(i915, 1); > if (max_latency == 0) > 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; @@ -2873,7 +2873,7 @@ > skl_compute_wm(struct intel_atomic_state *state) > struct intel_crtc *crtc; > struct intel_crtc_state __maybe_unused *new_crtc_state; > int ret, i; > - bool vrr_enabled = false; > + bool enable_dpkgc = false; > > for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { > ret = skl_build_pipe_wm(state, crtc); @@ -2899,11 +2899,13 > @@ skl_compute_wm(struct intel_atomic_state *state) > if (ret) > return ret; > > - if (new_crtc_state->vrr.enable) > - vrr_enabled = true; > + if ((new_crtc_state->vrr.vmin == new_crtc_state->vrr.vmax && > + new_crtc_state->vrr.vmin == new_crtc_state->vrr.flipline) || In current implementation this target to CMRR it seems. > + !new_crtc_state->vrr.enable) But in CMRR as well, vrr.enable is set along with vrr.enable. Any specific case you are targeting to ? > + enable_dpkgc = true; > } > > - skl_program_dpkgc_latency(to_i915(state->base.dev), vrr_enabled); > + skl_program_dpkgc_latency(to_i915(state->base.dev), enable_dpkgc); > > skl_print_wm_changes(state); > > -- > 2.43.2
> -----Original Message----- > From: Golani, Mitulkumar Ajitkumar > <mitulkumar.ajitkumar.golani@intel.com> > Sent: Thursday, July 18, 2024 9:35 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Manna, Animesh <animesh.manna@intel.com>; B, Jeevan > <jeevan.b@intel.com>; Naladala, Ramanaidu > <ramanaidu.naladala@intel.com> > Subject: RE: [PATCH] drm/i915/dpkgc: Add VRR condition for DPKGC > Enablement > > > > > -----Original Message----- > > From: Kandpal, Suraj <suraj.kandpal@intel.com> > > Sent: Thursday, July 11, 2024 10:19 AM > > To: intel-gfx@lists.freedesktop.org > > Cc: Manna, Animesh <animesh.manna@intel.com>; B, Jeevan > > <jeevan.b@intel.com>; Golani, Mitulkumar Ajitkumar > > <mitulkumar.ajitkumar.golani@intel.com>; Naladala, Ramanaidu > > <ramanaidu.naladala@intel.com>; Kandpal, Suraj > > <suraj.kandpal@intel.com> > > Subject: [PATCH] drm/i915/dpkgc: Add VRR condition for DPKGC > > Enablement > > > > DPKGC can now be enabled with VRR enabled if Vmin = Vmax = Flipline is > met. > > > > Bspec: 68986 > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > --- > > drivers/gpu/drm/i915/display/skl_watermark.c | 24 > > +++++++++++--------- > > 1 file changed, 13 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c > > b/drivers/gpu/drm/i915/display/skl_watermark.c > > index a2726364b34d..045c7cac166b 100644 > > --- a/drivers/gpu/drm/i915/display/skl_watermark.c > > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c > > @@ -2830,17 +2830,17 @@ static int skl_wm_add_affected_planes(struct > > intel_atomic_state *state, } > > > > /* > > - * If Fixed Refresh Rate: > > + * If Fixed Refresh Rate or For VRR case Vmin = Vmax = Flipline: > > * Program DEEP PKG_C_LATENCY Pkg C with highest valid latency from > > * watermark level1 and up and above. If watermark level 1 is > > * invalid program it with all 1's. > > * Program PKG_C_LATENCY Added Wake Time = DSB execution time > > - * If Variable Refresh Rate: > > + * If Variable Refresh Rate where Vmin != Vmax != Flipline: > > * Program DEEP PKG_C_LATENCY Pkg C with all 1's. > > * Program PKG_C_LATENCY Added Wake Time = 0 > > */ > > static void > > -skl_program_dpkgc_latency(struct drm_i915_private *i915, bool > > vrr_enabled) > > +skl_program_dpkgc_latency(struct drm_i915_private *i915, bool > > +enable_dpkgc) > > { > > u32 max_latency = 0; > > u32 clear = 0, val = 0; > > @@ -2849,15 +2849,15 @@ skl_program_dpkgc_latency(struct > > drm_i915_private *i915, bool vrr_enabled) > > if (DISPLAY_VER(i915) < 20) > > return; > > > > - if (vrr_enabled) { > > - max_latency = LNL_PKG_C_LATENCY_MASK; > > - added_wake_time = 0; > > - } else { > > + if (enable_dpkgc) { > > max_latency = skl_watermark_max_latency(i915, 1); > > if (max_latency == 0) > > 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; @@ -2873,7 +2873,7 @@ > skl_compute_wm(struct > > intel_atomic_state *state) > > struct intel_crtc *crtc; > > struct intel_crtc_state __maybe_unused *new_crtc_state; > > int ret, i; > > - bool vrr_enabled = false; > > + bool enable_dpkgc = false; > > > > for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { > > ret = skl_build_pipe_wm(state, crtc); @@ -2899,11 +2899,13 > @@ > > skl_compute_wm(struct intel_atomic_state *state) > > if (ret) > > return ret; > > > > - if (new_crtc_state->vrr.enable) > > - vrr_enabled = true; > > + if ((new_crtc_state->vrr.vmin == new_crtc_state->vrr.vmax && > > + new_crtc_state->vrr.vmin == new_crtc_state->vrr.flipline) > || > > In current implementation this target to CMRR it seems. Yes I saw Ankit's patch series which introduces vrr.is_fixed but I understand its not always set even if cmrr is in picture. Seems like this condition is the best as it best represents the condition mentioned in bspec. > > > + !new_crtc_state->vrr.enable) > But in CMRR as well, vrr.enable is set along with vrr.enable. Any > specific case you are targeting to ? So either we have vmax=vmin=flipline or vrr is disabled these are the two conditions in which Dpkgc can be enabled Regards, Suraj Kandpal > > > + enable_dpkgc = true; > > } > > > > - skl_program_dpkgc_latency(to_i915(state->base.dev), vrr_enabled); > > + skl_program_dpkgc_latency(to_i915(state->base.dev), > enable_dpkgc); > > > > > skl_print_wm_changes(state); > > > > -- > > 2.43.2
> -----Original Message----- > From: Kandpal, Suraj <suraj.kandpal@intel.com> > Sent: Monday, July 22, 2024 12:16 PM > To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>; > intel-gfx@lists.freedesktop.org > Cc: Manna, Animesh <animesh.manna@intel.com>; B, Jeevan > <jeevan.b@intel.com>; Naladala, Ramanaidu > <ramanaidu.naladala@intel.com> > Subject: RE: [PATCH] drm/i915/dpkgc: Add VRR condition for DPKGC > Enablement > > > > > -----Original Message----- > > From: Golani, Mitulkumar Ajitkumar > > <mitulkumar.ajitkumar.golani@intel.com> > > Sent: Thursday, July 18, 2024 9:35 PM > > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel- > > gfx@lists.freedesktop.org > > Cc: Manna, Animesh <animesh.manna@intel.com>; B, Jeevan > > <jeevan.b@intel.com>; Naladala, Ramanaidu > > <ramanaidu.naladala@intel.com> > > Subject: RE: [PATCH] drm/i915/dpkgc: Add VRR condition for DPKGC > > Enablement > > > > > > > > > -----Original Message----- > > > From: Kandpal, Suraj <suraj.kandpal@intel.com> > > > Sent: Thursday, July 11, 2024 10:19 AM > > > To: intel-gfx@lists.freedesktop.org > > > Cc: Manna, Animesh <animesh.manna@intel.com>; B, Jeevan > > > <jeevan.b@intel.com>; Golani, Mitulkumar Ajitkumar > > > <mitulkumar.ajitkumar.golani@intel.com>; Naladala, Ramanaidu > > > <ramanaidu.naladala@intel.com>; Kandpal, Suraj > > > <suraj.kandpal@intel.com> > > > Subject: [PATCH] drm/i915/dpkgc: Add VRR condition for DPKGC > > > Enablement > > > > > > DPKGC can now be enabled with VRR enabled if Vmin = Vmax = Flipline > > > is > > met. > > > > > > Bspec: 68986 > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/skl_watermark.c | 24 > > > +++++++++++--------- > > > 1 file changed, 13 insertions(+), 11 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c > > > b/drivers/gpu/drm/i915/display/skl_watermark.c > > > index a2726364b34d..045c7cac166b 100644 > > > --- a/drivers/gpu/drm/i915/display/skl_watermark.c > > > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c > > > @@ -2830,17 +2830,17 @@ static int > skl_wm_add_affected_planes(struct > > > intel_atomic_state *state, } > > > > > > /* > > > - * If Fixed Refresh Rate: > > > + * If Fixed Refresh Rate or For VRR case Vmin = Vmax = Flipline: > > > * Program DEEP PKG_C_LATENCY Pkg C with highest valid latency from > > > * watermark level1 and up and above. If watermark level 1 is > > > * invalid program it with all 1's. > > > * Program PKG_C_LATENCY Added Wake Time = DSB execution time > > > - * If Variable Refresh Rate: > > > + * If Variable Refresh Rate where Vmin != Vmax != Flipline: > > > * Program DEEP PKG_C_LATENCY Pkg C with all 1's. > > > * Program PKG_C_LATENCY Added Wake Time = 0 > > > */ > > > static void > > > -skl_program_dpkgc_latency(struct drm_i915_private *i915, bool > > > vrr_enabled) > > > +skl_program_dpkgc_latency(struct drm_i915_private *i915, bool > > > +enable_dpkgc) > > > { > > > u32 max_latency = 0; > > > u32 clear = 0, val = 0; > > > @@ -2849,15 +2849,15 @@ skl_program_dpkgc_latency(struct > > > drm_i915_private *i915, bool vrr_enabled) > > > if (DISPLAY_VER(i915) < 20) > > > return; > > > > > > - if (vrr_enabled) { > > > - max_latency = LNL_PKG_C_LATENCY_MASK; > > > - added_wake_time = 0; > > > - } else { > > > + if (enable_dpkgc) { > > > max_latency = skl_watermark_max_latency(i915, 1); > > > if (max_latency == 0) > > > 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; @@ > > > -2873,7 +2873,7 @@ > > skl_compute_wm(struct > > > intel_atomic_state *state) > > > struct intel_crtc *crtc; > > > struct intel_crtc_state __maybe_unused *new_crtc_state; > > > int ret, i; > > > - bool vrr_enabled = false; > > > + bool enable_dpkgc = false; > > > > > > for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { > > > ret = skl_build_pipe_wm(state, crtc); @@ -2899,11 +2899,13 > > @@ > > > skl_compute_wm(struct intel_atomic_state *state) > > > if (ret) > > > return ret; > > > > > > - if (new_crtc_state->vrr.enable) > > > - vrr_enabled = true; > > > + if ((new_crtc_state->vrr.vmin == new_crtc_state->vrr.vmax > && > > > + new_crtc_state->vrr.vmin == new_crtc_state->vrr.flipline) > > || > > > > In current implementation this target to CMRR it seems. > > Yes I saw Ankit's patch series which introduces vrr.is_fixed but I understand > its not always set even if cmrr is in picture. > Seems like this condition is the best as it best represents the condition > mentioned in bspec. > > > > > > + !new_crtc_state->vrr.enable) > > But in CMRR as well, vrr.enable is set along with vrr.enable. Any > > specific case you are targeting to ? > > So either we have vmax=vmin=flipline or vrr is disabled these are the two > conditions in which Dpkgc can be enabled > > Regards, > Suraj Kandpal > > > > > + enable_dpkgc = true; > > > } > > > > > > - skl_program_dpkgc_latency(to_i915(state->base.dev), vrr_enabled); > > > + skl_program_dpkgc_latency(to_i915(state->base.dev), > > enable_dpkgc); Changes LGTM. Reviewed-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> > > > > > > > > skl_print_wm_changes(state); > > > > > > -- > > > 2.43.2
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index a2726364b34d..045c7cac166b 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -2830,17 +2830,17 @@ static int skl_wm_add_affected_planes(struct intel_atomic_state *state, } /* - * If Fixed Refresh Rate: + * If Fixed Refresh Rate or For VRR case Vmin = Vmax = Flipline: * Program DEEP PKG_C_LATENCY Pkg C with highest valid latency from * watermark level1 and up and above. If watermark level 1 is * invalid program it with all 1's. * Program PKG_C_LATENCY Added Wake Time = DSB execution time - * If Variable Refresh Rate: + * If Variable Refresh Rate where Vmin != Vmax != Flipline: * Program DEEP PKG_C_LATENCY Pkg C with all 1's. * Program PKG_C_LATENCY Added Wake Time = 0 */ static void -skl_program_dpkgc_latency(struct drm_i915_private *i915, bool vrr_enabled) +skl_program_dpkgc_latency(struct drm_i915_private *i915, bool enable_dpkgc) { u32 max_latency = 0; u32 clear = 0, val = 0; @@ -2849,15 +2849,15 @@ skl_program_dpkgc_latency(struct drm_i915_private *i915, bool vrr_enabled) if (DISPLAY_VER(i915) < 20) return; - if (vrr_enabled) { - max_latency = LNL_PKG_C_LATENCY_MASK; - added_wake_time = 0; - } else { + if (enable_dpkgc) { max_latency = skl_watermark_max_latency(i915, 1); if (max_latency == 0) 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; @@ -2873,7 +2873,7 @@ skl_compute_wm(struct intel_atomic_state *state) struct intel_crtc *crtc; struct intel_crtc_state __maybe_unused *new_crtc_state; int ret, i; - bool vrr_enabled = false; + bool enable_dpkgc = false; for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { ret = skl_build_pipe_wm(state, crtc); @@ -2899,11 +2899,13 @@ skl_compute_wm(struct intel_atomic_state *state) if (ret) return ret; - if (new_crtc_state->vrr.enable) - vrr_enabled = true; + if ((new_crtc_state->vrr.vmin == new_crtc_state->vrr.vmax && + new_crtc_state->vrr.vmin == new_crtc_state->vrr.flipline) || + !new_crtc_state->vrr.enable) + enable_dpkgc = true; } - skl_program_dpkgc_latency(to_i915(state->base.dev), vrr_enabled); + skl_program_dpkgc_latency(to_i915(state->base.dev), enable_dpkgc); skl_print_wm_changes(state);
DPKGC can now be enabled with VRR enabled if Vmin = Vmax = Flipline is met. Bspec: 68986 Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> --- drivers/gpu/drm/i915/display/skl_watermark.c | 24 +++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-)