diff mbox series

[v4,1/5] drm/i915/vrr: Add crtc_state dump for vrr.vsync params

Message ID 20241120084948.1834306-2-mitulkumar.ajitkumar.golani@intel.com (mailing list archive)
State New
Headers show
Series Add AS_SDP to fastset | expand

Commit Message

Golani, Mitulkumar Ajitkumar Nov. 20, 2024, 8:49 a.m. UTC
Add crtc_state dump for vrr.vsync_{start/end} params to track the
state correctly.

Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
---
 drivers/gpu/drm/i915/display/intel_crtc_state_dump.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Jani Nikula Nov. 20, 2024, 12:50 p.m. UTC | #1
On Wed, 20 Nov 2024, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote:
> Add crtc_state dump for vrr.vsync_{start/end} params to track the
> state correctly.
>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_crtc_state_dump.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> index 705ec5ad385c..92dbf2cc150c 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> @@ -296,11 +296,13 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
>  		intel_dump_buffer("ELD: ", pipe_config->eld,
>  				  drm_eld_size(pipe_config->eld));
>  
> -	drm_printf(&p, "vrr: %s, vmin: %d, vmax: %d, pipeline full: %d, guardband: %d flipline: %d, vmin vblank: %d, vmax vblank: %d\n",
> +	drm_printf(&p, "vrr: %s, vmin: %d, vmax: %d, pipeline full: %d, guardband: %d flipline: %d, vrr_vsync_start: %d, vrr_vsync_end: %d, vmin vblank: %d, vmax vblank: %d\n",

Please look around you, and follow the patterns.

All the other fields here have spaces instead of underscores.

The whole line is all about VRR, there's no need to duplicate vrr in the
individual fields.

BR,
Jani.


>  		   str_yes_no(pipe_config->vrr.enable),
>  		   pipe_config->vrr.vmin, pipe_config->vrr.vmax,
>  		   pipe_config->vrr.pipeline_full, pipe_config->vrr.guardband,
>  		   pipe_config->vrr.flipline,
> +		   pipe_config->vrr.vsync_start,
> +		   pipe_config->vrr.vsync_end,
>  		   intel_vrr_vmin_vblank_start(pipe_config),
>  		   intel_vrr_vmax_vblank_start(pipe_config));
Golani, Mitulkumar Ajitkumar Nov. 21, 2024, 4:38 a.m. UTC | #2
> -----Original Message-----
> From: Nikula, Jani <jani.nikula@intel.com>
> Sent: 20 November 2024 18:20
> To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>;
> intel-gfx@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>;
> ville.syrjala@linux.intel.com
> Subject: Re: [PATCH v4 1/5] drm/i915/vrr: Add crtc_state dump for vrr.vsync
> params
> 
> On Wed, 20 Nov 2024, Mitul Golani
> <mitulkumar.ajitkumar.golani@intel.com> wrote:
> > Add crtc_state dump for vrr.vsync_{start/end} params to track the
> > state correctly.
> >
> > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_crtc_state_dump.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> > b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> > index 705ec5ad385c..92dbf2cc150c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> > +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> > @@ -296,11 +296,13 @@ void intel_crtc_state_dump(const struct
> intel_crtc_state *pipe_config,
> >  		intel_dump_buffer("ELD: ", pipe_config->eld,
> >  				  drm_eld_size(pipe_config->eld));
> >
> > -	drm_printf(&p, "vrr: %s, vmin: %d, vmax: %d, pipeline full: %d,
> guardband: %d flipline: %d, vmin vblank: %d, vmax vblank: %d\n",
> > +	drm_printf(&p, "vrr: %s, vmin: %d, vmax: %d, pipeline full: %d,
> > +guardband: %d flipline: %d, vrr_vsync_start: %d, vrr_vsync_end: %d,
> > +vmin vblank: %d, vmax vblank: %d\n",
> 
> Please look around you, and follow the patterns.
> 
> All the other fields here have spaces instead of underscores.
> 
> The whole line is all about VRR, there's no need to duplicate vrr in the
> individual fields.

Hi Jani,

I will update with suggested changes in next revision. Intention to add vrr as pretext to
Vsync_{start,end} was to avoid any confusion between TRANS_VSYNC and TRANS_VRR_VSYNC values.

Regards,
Mitul
> 
> BR,
> Jani.
> 
> 
> >  		   str_yes_no(pipe_config->vrr.enable),
> >  		   pipe_config->vrr.vmin, pipe_config->vrr.vmax,
> >  		   pipe_config->vrr.pipeline_full, pipe_config->vrr.guardband,
> >  		   pipe_config->vrr.flipline,
> > +		   pipe_config->vrr.vsync_start,
> > +		   pipe_config->vrr.vsync_end,
> >  		   intel_vrr_vmin_vblank_start(pipe_config),
> >  		   intel_vrr_vmax_vblank_start(pipe_config));
> 
> --
> Jani Nikula, Intel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
index 705ec5ad385c..92dbf2cc150c 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
@@ -296,11 +296,13 @@  void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
 		intel_dump_buffer("ELD: ", pipe_config->eld,
 				  drm_eld_size(pipe_config->eld));
 
-	drm_printf(&p, "vrr: %s, vmin: %d, vmax: %d, pipeline full: %d, guardband: %d flipline: %d, vmin vblank: %d, vmax vblank: %d\n",
+	drm_printf(&p, "vrr: %s, vmin: %d, vmax: %d, pipeline full: %d, guardband: %d flipline: %d, vrr_vsync_start: %d, vrr_vsync_end: %d, vmin vblank: %d, vmax vblank: %d\n",
 		   str_yes_no(pipe_config->vrr.enable),
 		   pipe_config->vrr.vmin, pipe_config->vrr.vmax,
 		   pipe_config->vrr.pipeline_full, pipe_config->vrr.guardband,
 		   pipe_config->vrr.flipline,
+		   pipe_config->vrr.vsync_start,
+		   pipe_config->vrr.vsync_end,
 		   intel_vrr_vmin_vblank_start(pipe_config),
 		   intel_vrr_vmax_vblank_start(pipe_config));