| Message ID | 20250326160321.550753-3-ankit.k.nautiyal@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | VRR Register Read/Write Updates | expand |
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
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
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,
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(-)