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

[Golani, Mitulkumar Ajitkumar]

> -----Original Message-----
> From: Nikula, Jani <jani.nikula at intel.com>
> Sent: 20 November 2024 18:20
> To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani at intel.com>;
> intel-gfx at lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal at intel.com>;
> ville.syrjala at 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 at 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 at 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