* [PATCH 0/2] VRR Register Read/Write Updates
@ 2025-03-26 16:03 Ankit Nautiyal
2025-03-26 16:03 ` [PATCH 1/2] drm/i915/display: Introduce transcoder_has_vrr() helper Ankit Nautiyal
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Ankit Nautiyal @ 2025-03-26 16:03 UTC (permalink / raw)
To: intel-gfx
Cc: intel-xe, jani.nikula, ville.syrjala, mitulkumar.ajitkumar.golani
Now that we have switched to VRR Timing generator from PTL onwards, we
no longer need to program VTOTAL.Vtotal bits, which were used by Legacy
Timing Generator.
This patch series is a continuation from discussion of another patch for
avoid reading/writing VTOTAL.Vtotal bits [1].
First patch introduces a macro to exclude DSI transcoded from VRR
programming in a consistent manner. The next patch actually modifies
reading/writing VTOTAL register.
[1] https://patchwork.freedesktop.org/patch/644683/?series=134383&rev=17
Rev2: Address comments from Ville.
Ankit Nautiyal (2):
drm/i915/display: Introduce transcoder_has_vrr() helper
drm/i915/display: Avoid use of VTOTAL.Vtotal bits
drivers/gpu/drm/i915/display/intel_display.c | 46 ++++++++++++++++++--
drivers/gpu/drm/i915/display/intel_vrr.c | 10 +++++
2 files changed, 52 insertions(+), 4 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/2] drm/i915/display: Introduce transcoder_has_vrr() helper 2025-03-26 16:03 [PATCH 0/2] VRR Register Read/Write Updates Ankit Nautiyal @ 2025-03-26 16:03 ` Ankit Nautiyal 2025-03-26 17:08 ` Ville Syrjälä 2025-03-26 16:03 ` [PATCH 2/2] drm/i915/display: Avoid use of VTOTAL.Vtotal bits Ankit Nautiyal 2025-03-26 17:06 ` ✗ i915.CI.BAT: failure for VRR Register Read/Write Updates (rev2) Patchwork 2 siblings, 1 reply; 11+ messages in thread From: Ankit Nautiyal @ 2025-03-26 16:03 UTC (permalink / raw) To: intel-gfx Cc: intel-xe, jani.nikula, ville.syrjala, mitulkumar.ajitkumar.golani To avoid having VRR read/write for DSI transcoders, we currently use !transcoder_is_dsi() in many places. Instead introduce a new helper to check transcoder_has_vrr() and use that to exclude transcoders which do not support VRR. v2: Include HAS_VRR into the helper. (Ville) Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index ee7812126129..0db1cd4fc963 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -2625,6 +2625,15 @@ void intel_cpu_transcoder_set_m2_n2(struct intel_crtc *crtc, PIPE_LINK_N2(display, transcoder)); } +static bool +transcoder_has_vrr(const struct intel_crtc_state *crtc_state) +{ + struct intel_display *display = to_intel_display(crtc_state); + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; + + return HAS_VRR(display) && !transcoder_is_dsi(cpu_transcoder); +} + static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_state) { struct intel_display *display = to_intel_display(crtc_state); @@ -2635,7 +2644,7 @@ static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_sta u32 crtc_vdisplay, crtc_vtotal, crtc_vblank_start, crtc_vblank_end; int vsyncshift = 0; - drm_WARN_ON(display->drm, transcoder_is_dsi(cpu_transcoder)); + drm_WARN_ON(display->drm, !transcoder_has_vrr(crtc_state)); /* We need to be careful not to changed the adjusted mode, for otherwise * the hw state checker will get angry at the mismatch. */ @@ -2717,7 +2726,7 @@ static void intel_set_transcoder_timings_lrr(const struct intel_crtc_state *crtc const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode; u32 crtc_vdisplay, crtc_vtotal, crtc_vblank_start, crtc_vblank_end; - drm_WARN_ON(display->drm, transcoder_is_dsi(cpu_transcoder)); + drm_WARN_ON(display->drm, !transcoder_has_vrr(crtc_state)); crtc_vdisplay = adjusted_mode->crtc_vdisplay; crtc_vtotal = adjusted_mode->crtc_vtotal; @@ -3908,7 +3917,7 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc, DISPLAY_VER(display) >= 11) intel_get_transcoder_timings(crtc, pipe_config); - if (HAS_VRR(display) && !transcoder_is_dsi(pipe_config->cpu_transcoder)) + if (transcoder_has_vrr(pipe_config)) intel_vrr_get_config(pipe_config); intel_get_pipe_src_size(crtc, pipe_config); -- 2.45.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] drm/i915/display: Introduce transcoder_has_vrr() helper 2025-03-26 16:03 ` [PATCH 1/2] drm/i915/display: Introduce transcoder_has_vrr() helper Ankit Nautiyal @ 2025-03-26 17:08 ` Ville Syrjälä 0 siblings, 0 replies; 11+ messages in thread From: Ville Syrjälä @ 2025-03-26 17:08 UTC (permalink / raw) To: Ankit Nautiyal Cc: intel-gfx, intel-xe, jani.nikula, mitulkumar.ajitkumar.golani On Wed, Mar 26, 2025 at 09:33:20PM +0530, Ankit Nautiyal wrote: > To avoid having VRR read/write for DSI transcoders, we currently use > !transcoder_is_dsi() in many places. > Instead introduce a new helper to check transcoder_has_vrr() and use > that to exclude transcoders which do not support VRR. > > v2: Include HAS_VRR into the helper. (Ville) > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index ee7812126129..0db1cd4fc963 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -2625,6 +2625,15 @@ void intel_cpu_transcoder_set_m2_n2(struct intel_crtc *crtc, > PIPE_LINK_N2(display, transcoder)); > } > > +static bool > +transcoder_has_vrr(const struct intel_crtc_state *crtc_state) > +{ > + struct intel_display *display = to_intel_display(crtc_state); > + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > + > + return HAS_VRR(display) && !transcoder_is_dsi(cpu_transcoder); > +} > + > static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_state) > { > struct intel_display *display = to_intel_display(crtc_state); > @@ -2635,7 +2644,7 @@ static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_sta > u32 crtc_vdisplay, crtc_vtotal, crtc_vblank_start, crtc_vblank_end; > int vsyncshift = 0; > > - drm_WARN_ON(display->drm, transcoder_is_dsi(cpu_transcoder)); > + drm_WARN_ON(display->drm, !transcoder_has_vrr(crtc_state)); Actually, this one should stay as is. We don't use this for DSI even on hardware that lacks VRR support. > > /* We need to be careful not to changed the adjusted mode, for otherwise > * the hw state checker will get angry at the mismatch. */ > @@ -2717,7 +2726,7 @@ static void intel_set_transcoder_timings_lrr(const struct intel_crtc_state *crtc > const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode; > u32 crtc_vdisplay, crtc_vtotal, crtc_vblank_start, crtc_vblank_end; > > - drm_WARN_ON(display->drm, transcoder_is_dsi(cpu_transcoder)); > + drm_WARN_ON(display->drm, !transcoder_has_vrr(crtc_state)); same here. > > crtc_vdisplay = adjusted_mode->crtc_vdisplay; > crtc_vtotal = adjusted_mode->crtc_vtotal; > @@ -3908,7 +3917,7 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc, > DISPLAY_VER(display) >= 11) > intel_get_transcoder_timings(crtc, pipe_config); > > - if (HAS_VRR(display) && !transcoder_is_dsi(pipe_config->cpu_transcoder)) > + if (transcoder_has_vrr(pipe_config)) > intel_vrr_get_config(pipe_config); This one is fine. > > intel_get_pipe_src_size(crtc, pipe_config); > -- > 2.45.2 -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] drm/i915/display: Avoid use of VTOTAL.Vtotal bits 2025-03-26 16:03 [PATCH 0/2] VRR Register Read/Write Updates Ankit Nautiyal 2025-03-26 16:03 ` [PATCH 1/2] drm/i915/display: Introduce transcoder_has_vrr() helper Ankit Nautiyal @ 2025-03-26 16:03 ` Ankit Nautiyal 2025-03-26 17:09 ` Ville Syrjälä 2025-03-26 17:06 ` ✗ i915.CI.BAT: failure for VRR Register Read/Write Updates (rev2) Patchwork 2 siblings, 1 reply; 11+ messages in thread From: Ankit Nautiyal @ 2025-03-26 16:03 UTC (permalink / raw) To: intel-gfx Cc: intel-xe, jani.nikula, ville.syrjala, mitulkumar.ajitkumar.golani For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal bits are not required. Since the support for these bits is going to be deprecated in upcoming platforms, avoid writing these bits for the platforms that do not use legacy Timing Generator. Since for these platforms TRAN_VMIN is always filled with crtc_vtotal, use TRAN_VRR_VMIN to get the vtotal for adjusted_mode. v2: Avoid having a helper for manipulating VTOTAL register, and instead just make the change where required. (Ville) v3: Set `crtc_vtotal` instead of working with the bits directly (Ville). Use intel_vrr_vmin_vtotal() to set the vtotal during readout. (Ville) Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 31 +++++++++++++++++++- drivers/gpu/drm/i915/display/intel_vrr.c | 10 +++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 0db1cd4fc963..6796dd0307a6 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -2698,9 +2698,19 @@ static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_sta HSYNC_START(adjusted_mode->crtc_hsync_start - 1) | HSYNC_END(adjusted_mode->crtc_hsync_end - 1)); + /* + * For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal + * bits are not required. Since the support for these bits is going to + * be deprecated in upcoming platforms, avoid writing these bits for the + * platforms that do not use legacy Timing Generator. + */ + if (intel_vrr_always_use_vrr_tg(display)) + crtc_vtotal = 1; + intel_de_write(display, TRANS_VTOTAL(display, cpu_transcoder), VACTIVE(crtc_vdisplay - 1) | VTOTAL(crtc_vtotal - 1)); + intel_de_write(display, TRANS_VBLANK(display, cpu_transcoder), VBLANK_START(crtc_vblank_start - 1) | VBLANK_END(crtc_vblank_end - 1)); @@ -2758,6 +2768,15 @@ static void intel_set_transcoder_timings_lrr(const struct intel_crtc_state *crtc intel_de_write(display, TRANS_VBLANK(display, cpu_transcoder), VBLANK_START(crtc_vblank_start - 1) | VBLANK_END(crtc_vblank_end - 1)); + /* + * For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal + * bits are not required. Since the support for these bits is going to + * be deprecated in upcoming platforms, avoid writing these bits for the + * platforms that do not use legacy Timing Generator. + */ + if (intel_vrr_always_use_vrr_tg(display)) + crtc_vtotal = 1; + /* * The double buffer latch point for TRANS_VTOTAL * is the transcoder's undelayed vblank. @@ -2827,7 +2846,17 @@ static void intel_get_transcoder_timings(struct intel_crtc *crtc, tmp = intel_de_read(display, TRANS_VTOTAL(display, cpu_transcoder)); adjusted_mode->crtc_vdisplay = REG_FIELD_GET(VACTIVE_MASK, tmp) + 1; - adjusted_mode->crtc_vtotal = REG_FIELD_GET(VTOTAL_MASK, tmp) + 1; + + /* + * For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal + * bits are not filled. The value for adjusted_mode->crtc_vtotal is read + * from VRR_VMIN register in intel_vrr_get_config. + * Just set this to 0 here. + */ + if (intel_vrr_always_use_vrr_tg(display)) + adjusted_mode->crtc_vtotal = 0; + else + adjusted_mode->crtc_vtotal = REG_FIELD_GET(VTOTAL_MASK, tmp) + 1; /* FIXME TGL+ DSI transcoders have this! */ if (!transcoder_is_dsi(cpu_transcoder)) { diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c index 414f93851059..7359d66fc091 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.c +++ b/drivers/gpu/drm/i915/display/intel_vrr.c @@ -708,6 +708,16 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state) crtc_state->vrr.vmin = intel_de_read(display, TRANS_VRR_VMIN(display, cpu_transcoder)) + 1; + /* + * For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal + * bits are not filled. Since for these platforms TRAN_VMIN is always + * filled with crtc_vtotal, use TRAN_VRR_VMIN to get the vtotal for + * adjusted_mode. + */ + if (intel_vrr_always_use_vrr_tg(display)) + crtc_state->hw.adjusted_mode.crtc_vtotal = + intel_vrr_vmin_vtotal(crtc_state); + if (HAS_AS_SDP(display)) { trans_vrr_vsync = intel_de_read(display, -- 2.45.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] drm/i915/display: Avoid use of VTOTAL.Vtotal bits 2025-03-26 16:03 ` [PATCH 2/2] drm/i915/display: Avoid use of VTOTAL.Vtotal bits Ankit Nautiyal @ 2025-03-26 17:09 ` Ville Syrjälä 2025-03-27 4:35 ` Nautiyal, Ankit K 0 siblings, 1 reply; 11+ messages in thread From: Ville Syrjälä @ 2025-03-26 17:09 UTC (permalink / raw) To: Ankit Nautiyal Cc: intel-gfx, intel-xe, jani.nikula, mitulkumar.ajitkumar.golani On Wed, Mar 26, 2025 at 09:33:21PM +0530, Ankit Nautiyal wrote: > For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal > bits are not required. Since the support for these bits is going to > be deprecated in upcoming platforms, avoid writing these bits for the > platforms that do not use legacy Timing Generator. > > Since for these platforms TRAN_VMIN is always filled with crtc_vtotal, > use TRAN_VRR_VMIN to get the vtotal for adjusted_mode. > > v2: Avoid having a helper for manipulating VTOTAL register, and instead > just make the change where required. (Ville) > v3: Set `crtc_vtotal` instead of working with the bits directly (Ville). > Use intel_vrr_vmin_vtotal() to set the vtotal during readout. (Ville) > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 31 +++++++++++++++++++- > drivers/gpu/drm/i915/display/intel_vrr.c | 10 +++++++ > 2 files changed, 40 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 0db1cd4fc963..6796dd0307a6 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -2698,9 +2698,19 @@ static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_sta > HSYNC_START(adjusted_mode->crtc_hsync_start - 1) | > HSYNC_END(adjusted_mode->crtc_hsync_end - 1)); > > + /* > + * For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal > + * bits are not required. Since the support for these bits is going to > + * be deprecated in upcoming platforms, avoid writing these bits for the > + * platforms that do not use legacy Timing Generator. > + */ > + if (intel_vrr_always_use_vrr_tg(display)) > + crtc_vtotal = 1; > + > intel_de_write(display, TRANS_VTOTAL(display, cpu_transcoder), > VACTIVE(crtc_vdisplay - 1) | > VTOTAL(crtc_vtotal - 1)); > + spurious whitespace change > intel_de_write(display, TRANS_VBLANK(display, cpu_transcoder), > VBLANK_START(crtc_vblank_start - 1) | > VBLANK_END(crtc_vblank_end - 1)); > @@ -2758,6 +2768,15 @@ static void intel_set_transcoder_timings_lrr(const struct intel_crtc_state *crtc > intel_de_write(display, TRANS_VBLANK(display, cpu_transcoder), > VBLANK_START(crtc_vblank_start - 1) | > VBLANK_END(crtc_vblank_end - 1)); > + /* > + * For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal > + * bits are not required. Since the support for these bits is going to > + * be deprecated in upcoming platforms, avoid writing these bits for the > + * platforms that do not use legacy Timing Generator. > + */ > + if (intel_vrr_always_use_vrr_tg(display)) > + crtc_vtotal = 1; > + > /* > * The double buffer latch point for TRANS_VTOTAL > * is the transcoder's undelayed vblank. > @@ -2827,7 +2846,17 @@ static void intel_get_transcoder_timings(struct intel_crtc *crtc, > > tmp = intel_de_read(display, TRANS_VTOTAL(display, cpu_transcoder)); > adjusted_mode->crtc_vdisplay = REG_FIELD_GET(VACTIVE_MASK, tmp) + 1; > - adjusted_mode->crtc_vtotal = REG_FIELD_GET(VTOTAL_MASK, tmp) + 1; > + > + /* > + * For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal > + * bits are not filled. The value for adjusted_mode->crtc_vtotal is read > + * from VRR_VMIN register in intel_vrr_get_config. > + * Just set this to 0 here. > + */ > + if (intel_vrr_always_use_vrr_tg(display)) This one either needs the transcoder_has_vrr() check, or we could just keep on blindly reading this anyway, and let intel_vrr_get_config() overwrite it afterwards. That's kinda how we deal with TRANS_SET_CONTEXT_LATENCY as well. > + adjusted_mode->crtc_vtotal = 0; > + else > + adjusted_mode->crtc_vtotal = REG_FIELD_GET(VTOTAL_MASK, tmp) + 1; > > /* FIXME TGL+ DSI transcoders have this! */ > if (!transcoder_is_dsi(cpu_transcoder)) { > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c > index 414f93851059..7359d66fc091 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > @@ -708,6 +708,16 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state) > crtc_state->vrr.vmin = intel_de_read(display, > TRANS_VRR_VMIN(display, cpu_transcoder)) + 1; > > + /* > + * For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal > + * bits are not filled. Since for these platforms TRAN_VMIN is always > + * filled with crtc_vtotal, use TRAN_VRR_VMIN to get the vtotal for > + * adjusted_mode. > + */ > + if (intel_vrr_always_use_vrr_tg(display)) > + crtc_state->hw.adjusted_mode.crtc_vtotal = > + intel_vrr_vmin_vtotal(crtc_state); > + > if (HAS_AS_SDP(display)) { > trans_vrr_vsync = > intel_de_read(display, > -- > 2.45.2 -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] drm/i915/display: Avoid use of VTOTAL.Vtotal bits 2025-03-26 17:09 ` Ville Syrjälä @ 2025-03-27 4:35 ` Nautiyal, Ankit K 0 siblings, 0 replies; 11+ messages in thread From: Nautiyal, Ankit K @ 2025-03-27 4:35 UTC (permalink / raw) To: Ville Syrjälä Cc: intel-gfx, intel-xe, jani.nikula, mitulkumar.ajitkumar.golani On 3/26/2025 10:39 PM, Ville Syrjälä wrote: > On Wed, Mar 26, 2025 at 09:33:21PM +0530, Ankit Nautiyal wrote: >> For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal >> bits are not required. Since the support for these bits is going to >> be deprecated in upcoming platforms, avoid writing these bits for the >> platforms that do not use legacy Timing Generator. >> >> Since for these platforms TRAN_VMIN is always filled with crtc_vtotal, >> use TRAN_VRR_VMIN to get the vtotal for adjusted_mode. >> >> v2: Avoid having a helper for manipulating VTOTAL register, and instead >> just make the change where required. (Ville) >> v3: Set `crtc_vtotal` instead of working with the bits directly (Ville). >> Use intel_vrr_vmin_vtotal() to set the vtotal during readout. (Ville) >> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_display.c | 31 +++++++++++++++++++- >> drivers/gpu/drm/i915/display/intel_vrr.c | 10 +++++++ >> 2 files changed, 40 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c >> index 0db1cd4fc963..6796dd0307a6 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display.c >> +++ b/drivers/gpu/drm/i915/display/intel_display.c >> @@ -2698,9 +2698,19 @@ static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_sta >> HSYNC_START(adjusted_mode->crtc_hsync_start - 1) | >> HSYNC_END(adjusted_mode->crtc_hsync_end - 1)); >> >> + /* >> + * For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal >> + * bits are not required. Since the support for these bits is going to >> + * be deprecated in upcoming platforms, avoid writing these bits for the >> + * platforms that do not use legacy Timing Generator. >> + */ >> + if (intel_vrr_always_use_vrr_tg(display)) >> + crtc_vtotal = 1; >> + >> intel_de_write(display, TRANS_VTOTAL(display, cpu_transcoder), >> VACTIVE(crtc_vdisplay - 1) | >> VTOTAL(crtc_vtotal - 1)); >> + > spurious whitespace change > >> intel_de_write(display, TRANS_VBLANK(display, cpu_transcoder), >> VBLANK_START(crtc_vblank_start - 1) | >> VBLANK_END(crtc_vblank_end - 1)); >> @@ -2758,6 +2768,15 @@ static void intel_set_transcoder_timings_lrr(const struct intel_crtc_state *crtc >> intel_de_write(display, TRANS_VBLANK(display, cpu_transcoder), >> VBLANK_START(crtc_vblank_start - 1) | >> VBLANK_END(crtc_vblank_end - 1)); >> + /* >> + * For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal >> + * bits are not required. Since the support for these bits is going to >> + * be deprecated in upcoming platforms, avoid writing these bits for the >> + * platforms that do not use legacy Timing Generator. >> + */ >> + if (intel_vrr_always_use_vrr_tg(display)) >> + crtc_vtotal = 1; >> + >> /* >> * The double buffer latch point for TRANS_VTOTAL >> * is the transcoder's undelayed vblank. >> @@ -2827,7 +2846,17 @@ static void intel_get_transcoder_timings(struct intel_crtc *crtc, >> >> tmp = intel_de_read(display, TRANS_VTOTAL(display, cpu_transcoder)); >> adjusted_mode->crtc_vdisplay = REG_FIELD_GET(VACTIVE_MASK, tmp) + 1; >> - adjusted_mode->crtc_vtotal = REG_FIELD_GET(VTOTAL_MASK, tmp) + 1; >> + >> + /* >> + * For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal >> + * bits are not filled. The value for adjusted_mode->crtc_vtotal is read >> + * from VRR_VMIN register in intel_vrr_get_config. >> + * Just set this to 0 here. >> + */ >> + if (intel_vrr_always_use_vrr_tg(display)) > This one either needs the transcoder_has_vrr() check, or we could just > keep on blindly reading this anyway, and let intel_vrr_get_config() > overwrite it afterwards. That's kinda how we deal with > TRANS_SET_CONTEXT_LATENCY as well. Understood. I think will just let vtotal be filled here and for platforms that always use VRR TG it will get overwritten in intel_vrr_get_config. Regards, Ankit > >> + adjusted_mode->crtc_vtotal = 0; >> + else >> + adjusted_mode->crtc_vtotal = REG_FIELD_GET(VTOTAL_MASK, tmp) + 1; >> >> /* FIXME TGL+ DSI transcoders have this! */ >> if (!transcoder_is_dsi(cpu_transcoder)) { >> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c >> index 414f93851059..7359d66fc091 100644 >> --- a/drivers/gpu/drm/i915/display/intel_vrr.c >> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c >> @@ -708,6 +708,16 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state) >> crtc_state->vrr.vmin = intel_de_read(display, >> TRANS_VRR_VMIN(display, cpu_transcoder)) + 1; >> >> + /* >> + * For platforms that always use VRR Timing Generator, the VTOTAL.Vtotal >> + * bits are not filled. Since for these platforms TRAN_VMIN is always >> + * filled with crtc_vtotal, use TRAN_VRR_VMIN to get the vtotal for >> + * adjusted_mode. >> + */ >> + if (intel_vrr_always_use_vrr_tg(display)) >> + crtc_state->hw.adjusted_mode.crtc_vtotal = >> + intel_vrr_vmin_vtotal(crtc_state); >> + >> if (HAS_AS_SDP(display)) { >> trans_vrr_vsync = >> intel_de_read(display, >> -- >> 2.45.2 ^ permalink raw reply [flat|nested] 11+ messages in thread
* ✗ i915.CI.BAT: failure for VRR Register Read/Write Updates (rev2) 2025-03-26 16:03 [PATCH 0/2] VRR Register Read/Write Updates Ankit Nautiyal 2025-03-26 16:03 ` [PATCH 1/2] drm/i915/display: Introduce transcoder_has_vrr() helper Ankit Nautiyal 2025-03-26 16:03 ` [PATCH 2/2] drm/i915/display: Avoid use of VTOTAL.Vtotal bits Ankit Nautiyal @ 2025-03-26 17:06 ` Patchwork 2 siblings, 0 replies; 11+ messages in thread From: Patchwork @ 2025-03-26 17:06 UTC (permalink / raw) To: Ankit Nautiyal; +Cc: intel-gfx [-- Attachment #1: Type: text/plain, Size: 8651 bytes --] == Series Details == Series: VRR Register Read/Write Updates (rev2) URL : https://patchwork.freedesktop.org/series/146778/ State : failure == Summary == CI Bug Log - changes from CI_DRM_16321 -> Patchwork_146778v2 ==================================================== Summary ------- **FAILURE** Serious unknown changes coming with Patchwork_146778v2 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_146778v2, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/index.html Participating hosts (40 -> 40) ------------------------------ Additional (2): bat-jsl-4 fi-pnv-d510 Missing (2): bat-twl-1 fi-snb-2520m Possible new issues ------------------- Here are the unknown changes that may have been introduced in Patchwork_146778v2: ### IGT changes ### #### Possible regressions #### * igt@i915_module_load@load: - fi-ilk-650: [PASS][1] -> [ABORT][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16321/fi-ilk-650/igt@i915_module_load@load.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/fi-ilk-650/igt@i915_module_load@load.html - fi-blb-e6850: [PASS][3] -> [ABORT][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16321/fi-blb-e6850/igt@i915_module_load@load.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/fi-blb-e6850/igt@i915_module_load@load.html - fi-bsw-n3050: [PASS][5] -> [ABORT][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16321/fi-bsw-n3050/igt@i915_module_load@load.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/fi-bsw-n3050/igt@i915_module_load@load.html - fi-pnv-d510: NOTRUN -> [ABORT][7] [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/fi-pnv-d510/igt@i915_module_load@load.html - fi-kbl-7567u: [PASS][8] -> [ABORT][9] [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16321/fi-kbl-7567u/igt@i915_module_load@load.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/fi-kbl-7567u/igt@i915_module_load@load.html - fi-cfl-8700k: [PASS][10] -> [ABORT][11] [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16321/fi-cfl-8700k/igt@i915_module_load@load.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/fi-cfl-8700k/igt@i915_module_load@load.html - bat-apl-1: [PASS][12] -> [ABORT][13] [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16321/bat-apl-1/igt@i915_module_load@load.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/bat-apl-1/igt@i915_module_load@load.html - fi-elk-e7500: [PASS][14] -> [ABORT][15] [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16321/fi-elk-e7500/igt@i915_module_load@load.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/fi-elk-e7500/igt@i915_module_load@load.html - fi-bsw-nick: [PASS][16] -> [ABORT][17] [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16321/fi-bsw-nick/igt@i915_module_load@load.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/fi-bsw-nick/igt@i915_module_load@load.html - fi-cfl-guc: [PASS][18] -> [ABORT][19] [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16321/fi-cfl-guc/igt@i915_module_load@load.html [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/fi-cfl-guc/igt@i915_module_load@load.html - fi-hsw-4770: [PASS][20] -> [ABORT][21] [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16321/fi-hsw-4770/igt@i915_module_load@load.html [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/fi-hsw-4770/igt@i915_module_load@load.html - fi-ivb-3770: [PASS][22] -> [ABORT][23] [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16321/fi-ivb-3770/igt@i915_module_load@load.html [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/fi-ivb-3770/igt@i915_module_load@load.html * igt@kms_busy@basic: - fi-skl-6600u: [PASS][24] -> [ABORT][25] +1 other test abort [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16321/fi-skl-6600u/igt@kms_busy@basic.html [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/fi-skl-6600u/igt@kms_busy@basic.html * igt@kms_force_connector_basic@force-connector-state: - bat-kbl-2: [PASS][26] -> [ABORT][27] [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16321/bat-kbl-2/igt@kms_force_connector_basic@force-connector-state.html [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/bat-kbl-2/igt@kms_force_connector_basic@force-connector-state.html - fi-kbl-x1275: [PASS][28] -> [ABORT][29] [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16321/fi-kbl-x1275/igt@kms_force_connector_basic@force-connector-state.html [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/fi-kbl-x1275/igt@kms_force_connector_basic@force-connector-state.html - fi-kbl-8809g: [PASS][30] -> [ABORT][31] [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16321/fi-kbl-8809g/igt@kms_force_connector_basic@force-connector-state.html [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/fi-kbl-8809g/igt@kms_force_connector_basic@force-connector-state.html - fi-kbl-guc: [PASS][32] -> [ABORT][33] [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16321/fi-kbl-guc/igt@kms_force_connector_basic@force-connector-state.html [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/fi-kbl-guc/igt@kms_force_connector_basic@force-connector-state.html #### Warnings #### * igt@i915_module_load@load: - fi-cfl-8109u: [DMESG-WARN][34] ([i915#13735]) -> [ABORT][35] [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16321/fi-cfl-8109u/igt@i915_module_load@load.html [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/fi-cfl-8109u/igt@i915_module_load@load.html #### Suppressed #### The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@i915_module_load@load: - {bat-jsl-4}: NOTRUN -> [INCOMPLETE][36] [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/bat-jsl-4/igt@i915_module_load@load.html Known issues ------------ Here are the changes found in Patchwork_146778v2 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@kms_chamelium_hpd@vga-hpd-fast: - bat-dg2-13: NOTRUN -> [SKIP][37] ([Intel XE#484] / [i915#4550]) +1 other test skip [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/bat-dg2-13/igt@kms_chamelium_hpd@vga-hpd-fast.html #### Possible fixes #### * igt@i915_selftest@live: - bat-adlp-11: [ABORT][38] ([i915#13696]) -> [PASS][39] +1 other test pass [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16321/bat-adlp-11/igt@i915_selftest@live.html [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/bat-adlp-11/igt@i915_selftest@live.html * igt@kms_chamelium_edid@dp-edid-read: - bat-dg2-13: [ABORT][40] ([i915#13940]) -> [PASS][41] [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16321/bat-dg2-13/igt@kms_chamelium_edid@dp-edid-read.html [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/bat-dg2-13/igt@kms_chamelium_edid@dp-edid-read.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [Intel XE#484]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/484 [i915#13696]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13696 [i915#13735]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13735 [i915#13940]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13940 [i915#4550]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4550 Build changes ------------- * Linux: CI_DRM_16321 -> Patchwork_146778v2 CI-20190529: 20190529 CI_DRM_16321: 14c330bc015ded4a1f1dd1f5aeb8617077aaa7e8 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_8284: 8284 Patchwork_146778v2: 14c330bc015ded4a1f1dd1f5aeb8617077aaa7e8 @ git://anongit.freedesktop.org/gfx-ci/linux == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_146778v2/index.html [-- Attachment #2: Type: text/html, Size: 9532 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 0/2] VRR Register Read/Write Updates @ 2025-03-27 14:46 Ankit Nautiyal 2025-03-27 14:46 ` [PATCH 1/2] drm/i915/display: Introduce transcoder_has_vrr() helper Ankit Nautiyal 0 siblings, 1 reply; 11+ messages in thread From: Ankit Nautiyal @ 2025-03-27 14:46 UTC (permalink / raw) To: intel-gfx Cc: intel-xe, jani.nikula, ville.syrjala, mitulkumar.ajitkumar.golani Now that we have switched to VRR Timing generator from PTL onwards, we no longer need to program VTOTAL.Vtotal bits, which were used by Legacy Timing Generator. This patch series is a continuation from discussion of another patch for avoid reading/writing VTOTAL.Vtotal bits [1]. First patch introduces a macro to exclude DSI transcoded from VRR programming in a consistent manner. The next patch actually modifies reading/writing VTOTAL register. [1] https://patchwork.freedesktop.org/patch/644683/?series=134383&rev=17 Rev2: Address comments from Ville. Rev3: Fix the BAT issues due to incorrect check. Keep the readout for vtotal intact, and just overwrite for cases where we dont want to read vtotal. (Ville) Ankit Nautiyal (2): drm/i915/display: Introduce transcoder_has_vrr() helper drm/i915/display: Avoid use of VTOTAL.Vtotal bits drivers/gpu/drm/i915/display/intel_display.c | 29 +++++++++++++++++++- drivers/gpu/drm/i915/display/intel_vrr.c | 10 +++++++ 2 files changed, 38 insertions(+), 1 deletion(-) -- 2.45.2 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] drm/i915/display: Introduce transcoder_has_vrr() helper 2025-03-27 14:46 [PATCH 0/2] VRR Register Read/Write Updates Ankit Nautiyal @ 2025-03-27 14:46 ` Ankit Nautiyal 2025-03-28 10:35 ` Ville Syrjälä 0 siblings, 1 reply; 11+ messages in thread From: Ankit Nautiyal @ 2025-03-27 14:46 UTC (permalink / raw) To: intel-gfx Cc: intel-xe, jani.nikula, ville.syrjala, mitulkumar.ajitkumar.golani Introduce a new helper to check transcoder_has_vrr() and use that to exclude transcoders which do not support VRR. v2: Include HAS_VRR into the helper. (Ville) v3: Drop the usage in places where not applicable. (Ville) Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index ee7812126129..b82b3d63be73 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -2625,6 +2625,15 @@ void intel_cpu_transcoder_set_m2_n2(struct intel_crtc *crtc, PIPE_LINK_N2(display, transcoder)); } +static bool +transcoder_has_vrr(const struct intel_crtc_state *crtc_state) +{ + struct intel_display *display = to_intel_display(crtc_state); + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; + + return HAS_VRR(display) && !transcoder_is_dsi(cpu_transcoder); +} + static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_state) { struct intel_display *display = to_intel_display(crtc_state); @@ -3908,7 +3917,7 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc, DISPLAY_VER(display) >= 11) intel_get_transcoder_timings(crtc, pipe_config); - if (HAS_VRR(display) && !transcoder_is_dsi(pipe_config->cpu_transcoder)) + if (transcoder_has_vrr(pipe_config)) intel_vrr_get_config(pipe_config); intel_get_pipe_src_size(crtc, pipe_config); -- 2.45.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] drm/i915/display: Introduce transcoder_has_vrr() helper 2025-03-27 14:46 ` [PATCH 1/2] drm/i915/display: Introduce transcoder_has_vrr() helper Ankit Nautiyal @ 2025-03-28 10:35 ` Ville Syrjälä 0 siblings, 0 replies; 11+ messages in thread From: Ville Syrjälä @ 2025-03-28 10:35 UTC (permalink / raw) To: Ankit Nautiyal Cc: intel-gfx, intel-xe, jani.nikula, mitulkumar.ajitkumar.golani On Thu, Mar 27, 2025 at 08:16:28PM +0530, Ankit Nautiyal wrote: > Introduce a new helper to check transcoder_has_vrr() and use > that to exclude transcoders which do not support VRR. > > v2: Include HAS_VRR into the helper. (Ville) > v3: Drop the usage in places where not applicable. (Ville) > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index ee7812126129..b82b3d63be73 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -2625,6 +2625,15 @@ void intel_cpu_transcoder_set_m2_n2(struct intel_crtc *crtc, > PIPE_LINK_N2(display, transcoder)); > } > > +static bool > +transcoder_has_vrr(const struct intel_crtc_state *crtc_state) > +{ > + struct intel_display *display = to_intel_display(crtc_state); > + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > + > + return HAS_VRR(display) && !transcoder_is_dsi(cpu_transcoder); > +} > + > static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_state) > { > struct intel_display *display = to_intel_display(crtc_state); > @@ -3908,7 +3917,7 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc, > DISPLAY_VER(display) >= 11) > intel_get_transcoder_timings(crtc, pipe_config); > > - if (HAS_VRR(display) && !transcoder_is_dsi(pipe_config->cpu_transcoder)) > + if (transcoder_has_vrr(pipe_config)) > intel_vrr_get_config(pipe_config); > > intel_get_pipe_src_size(crtc, pipe_config); > -- > 2.45.2 -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 0/2] VRR Register Read/Write Updates @ 2025-03-26 4:05 Ankit Nautiyal 2025-03-26 4:05 ` [PATCH 1/2] drm/i915/display: Introduce transcoder_has_vrr() helper Ankit Nautiyal 0 siblings, 1 reply; 11+ messages in thread From: Ankit Nautiyal @ 2025-03-26 4:05 UTC (permalink / raw) To: intel-gfx Cc: intel-xe, jani.nikula, ville.syrjala, mitulkumar.ajitkumar.golani Now that we have switched to VRR Timing generator from PTL onwards, we no longer need to program VTOTAL.Vtotal bits, which were used by Legacy Timing Generator. This patch series is a continuation from discussion of another patch for avoid reading/writing VTOTAL.Vtotal bits [1]. First patch introduces a macro to exclude DSI transcoded from VRR programming in a consistent manner. The next patch actually modifies reading/writing VTOTAL register. [1] https://patchwork.freedesktop.org/patch/644683/?series=134383&rev=17 Ankit Nautiyal (2): drm/i915/display: Introduce transcoder_has_vrr() helper drm/i915/display: Avoid use of VTOTAL.Vtotal bits drivers/gpu/drm/i915/display/intel_display.c | 53 +++++++++++++++++--- drivers/gpu/drm/i915/display/intel_vrr.c | 15 ++++-- 2 files changed, 59 insertions(+), 9 deletions(-) -- 2.45.2 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] drm/i915/display: Introduce transcoder_has_vrr() helper 2025-03-26 4:05 [PATCH 0/2] VRR Register Read/Write Updates Ankit Nautiyal @ 2025-03-26 4:05 ` Ankit Nautiyal 2025-03-26 13:01 ` Ville Syrjälä 0 siblings, 1 reply; 11+ messages in thread From: Ankit Nautiyal @ 2025-03-26 4:05 UTC (permalink / raw) To: intel-gfx Cc: intel-xe, jani.nikula, ville.syrjala, mitulkumar.ajitkumar.golani To avoid having VRR read/write for DSI transcoders, we currently use !transcoder_is_dsi() in many places. Instead introduce a new helper to check transcoder_has_vrr() and use that to exclude transcoders which do not support VRR. Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index ee7812126129..bde53b2de70c 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -2625,6 +2625,12 @@ void intel_cpu_transcoder_set_m2_n2(struct intel_crtc *crtc, PIPE_LINK_N2(display, transcoder)); } +static bool +transcoder_has_vrr(enum transcoder cpu_transcoder) +{ + return !transcoder_is_dsi(cpu_transcoder); +} + static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_state) { struct intel_display *display = to_intel_display(crtc_state); @@ -2635,7 +2641,7 @@ static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_sta u32 crtc_vdisplay, crtc_vtotal, crtc_vblank_start, crtc_vblank_end; int vsyncshift = 0; - drm_WARN_ON(display->drm, transcoder_is_dsi(cpu_transcoder)); + drm_WARN_ON(display->drm, !transcoder_has_vrr(cpu_transcoder)); /* We need to be careful not to changed the adjusted mode, for otherwise * the hw state checker will get angry at the mismatch. */ @@ -2717,7 +2723,7 @@ static void intel_set_transcoder_timings_lrr(const struct intel_crtc_state *crtc const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode; u32 crtc_vdisplay, crtc_vtotal, crtc_vblank_start, crtc_vblank_end; - drm_WARN_ON(display->drm, transcoder_is_dsi(cpu_transcoder)); + drm_WARN_ON(display->drm, !transcoder_has_vrr(cpu_transcoder)); crtc_vdisplay = adjusted_mode->crtc_vdisplay; crtc_vtotal = adjusted_mode->crtc_vtotal; @@ -3908,7 +3914,7 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc, DISPLAY_VER(display) >= 11) intel_get_transcoder_timings(crtc, pipe_config); - if (HAS_VRR(display) && !transcoder_is_dsi(pipe_config->cpu_transcoder)) + if (HAS_VRR(display) && transcoder_has_vrr(pipe_config->cpu_transcoder)) intel_vrr_get_config(pipe_config); intel_get_pipe_src_size(crtc, pipe_config); -- 2.45.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] drm/i915/display: Introduce transcoder_has_vrr() helper 2025-03-26 4:05 ` [PATCH 1/2] drm/i915/display: Introduce transcoder_has_vrr() helper Ankit Nautiyal @ 2025-03-26 13:01 ` Ville Syrjälä 0 siblings, 0 replies; 11+ messages in thread From: Ville Syrjälä @ 2025-03-26 13:01 UTC (permalink / raw) To: Ankit Nautiyal Cc: intel-gfx, intel-xe, jani.nikula, mitulkumar.ajitkumar.golani On Wed, Mar 26, 2025 at 09:35:37AM +0530, Ankit Nautiyal wrote: > To avoid having VRR read/write for DSI transcoders, we currently use > !transcoder_is_dsi() in many places. > Instead introduce a new helper to check transcoder_has_vrr() and use > that to exclude transcoders which do not support VRR. > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index ee7812126129..bde53b2de70c 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -2625,6 +2625,12 @@ void intel_cpu_transcoder_set_m2_n2(struct intel_crtc *crtc, > PIPE_LINK_N2(display, transcoder)); > } > > +static bool > +transcoder_has_vrr(enum transcoder cpu_transcoder) > +{ I would put the HAS_VRR() check in here as well. > + return !transcoder_is_dsi(cpu_transcoder); > +} > + > static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_state) > { > struct intel_display *display = to_intel_display(crtc_state); > @@ -2635,7 +2641,7 @@ static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_sta > u32 crtc_vdisplay, crtc_vtotal, crtc_vblank_start, crtc_vblank_end; > int vsyncshift = 0; > > - drm_WARN_ON(display->drm, transcoder_is_dsi(cpu_transcoder)); > + drm_WARN_ON(display->drm, !transcoder_has_vrr(cpu_transcoder)); > > /* We need to be careful not to changed the adjusted mode, for otherwise > * the hw state checker will get angry at the mismatch. */ > @@ -2717,7 +2723,7 @@ static void intel_set_transcoder_timings_lrr(const struct intel_crtc_state *crtc > const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode; > u32 crtc_vdisplay, crtc_vtotal, crtc_vblank_start, crtc_vblank_end; > > - drm_WARN_ON(display->drm, transcoder_is_dsi(cpu_transcoder)); > + drm_WARN_ON(display->drm, !transcoder_has_vrr(cpu_transcoder)); > > crtc_vdisplay = adjusted_mode->crtc_vdisplay; > crtc_vtotal = adjusted_mode->crtc_vtotal; > @@ -3908,7 +3914,7 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc, > DISPLAY_VER(display) >= 11) > intel_get_transcoder_timings(crtc, pipe_config); > > - if (HAS_VRR(display) && !transcoder_is_dsi(pipe_config->cpu_transcoder)) > + if (HAS_VRR(display) && transcoder_has_vrr(pipe_config->cpu_transcoder)) > intel_vrr_get_config(pipe_config); > > intel_get_pipe_src_size(crtc, pipe_config); > -- > 2.45.2 -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-03-28 10:35 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-03-26 16:03 [PATCH 0/2] VRR Register Read/Write Updates Ankit Nautiyal 2025-03-26 16:03 ` [PATCH 1/2] drm/i915/display: Introduce transcoder_has_vrr() helper Ankit Nautiyal 2025-03-26 17:08 ` Ville Syrjälä 2025-03-26 16:03 ` [PATCH 2/2] drm/i915/display: Avoid use of VTOTAL.Vtotal bits Ankit Nautiyal 2025-03-26 17:09 ` Ville Syrjälä 2025-03-27 4:35 ` Nautiyal, Ankit K 2025-03-26 17:06 ` ✗ i915.CI.BAT: failure for VRR Register Read/Write Updates (rev2) Patchwork -- strict thread matches above, loose matches on Subject: below -- 2025-03-27 14:46 [PATCH 0/2] VRR Register Read/Write Updates Ankit Nautiyal 2025-03-27 14:46 ` [PATCH 1/2] drm/i915/display: Introduce transcoder_has_vrr() helper Ankit Nautiyal 2025-03-28 10:35 ` Ville Syrjälä 2025-03-26 4:05 [PATCH 0/2] VRR Register Read/Write Updates Ankit Nautiyal 2025-03-26 4:05 ` [PATCH 1/2] drm/i915/display: Introduce transcoder_has_vrr() helper Ankit Nautiyal 2025-03-26 13:01 ` Ville Syrjälä
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox