[Intel-gfx] [PATCH 2/2] drm/i915/display/debug: Expose Dither status via debugfs

[Modem, Bhanuprakash]

> From: Jani Nikula <jani.nikula at linux.intel.com>
> Sent: Wednesday, May 26, 2021 7:57 PM
> To: Modem, Bhanuprakash <bhanuprakash.modem at intel.com>; intel-
> gfx at lists.freedesktop.org; Varide, Nischal <nischal.varide at intel.com>;
> Shankar, Uma <uma.shankar at intel.com>; Gupta, Anshuman
> <anshuman.gupta at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915/display/debug: Expose Dither
> status via debugfs
> 
> On Wed, 26 May 2021, Bhanuprakash Modem <bhanuprakash.modem at intel.com> wrote:
> > It's useful to know the dithering state & pipe bpc for IGT testing.
> > This patch will expose the dithering state for the crtc via a debugfs
> > file "dither".
> >
> > Example usage: cat /sys/kernel/debug/dri/0/crtc-0/dither
> >
> > Cc: Uma Shankar <uma.shankar at intel.com>
> > Cc: Nischal Varide <nischal.varide at intel.com>
> > Cc: Matt Roper <matthew.d.roper at intel.com>
> > Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem at intel.com>
> > ---
> >  .../drm/i915/display/intel_display_debugfs.c  | 32 +++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > index 94e5cbd86e77..a6fefc7d5ab9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > @@ -2158,11 +2158,43 @@ static const struct {
> >  	{"i915_edp_psr_debug", &i915_edp_psr_debug_fops},
> >  };
> >
> > +static int dither_state_show(struct seq_file *m, void *data)
> > +{
> > +	struct intel_crtc *crtc = to_intel_crtc(m->private);
> > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > +	struct intel_crtc_state *crtc_state;
> > +	int ret;
> > +
> > +	if (!HAS_DISPLAY(dev_priv))
> > +		return -ENODEV;
> 
> Unneeded.
> 
> > +
> > +	ret = drm_modeset_lock_single_interruptible(&crtc->base.mutex);
> > +	if (ret)
> > +		return ret;
> > +
> > +	crtc_state = to_intel_crtc_state(crtc->base.state);
> > +	seq_printf(m, "bpc: %u\n", crtc_state->pipe_bpp / 3);
> > +	seq_printf(m, "Dither: %u\n", (crtc_state->dither) ? 1 : 0);
> > +	seq_printf(m, "Dither_CC1: %u\n",
> > +		(crtc_state->gamma_mode & GAMMA_MODE_DITHER_AFTER_CC1) ? 1 : 0);
> 
> Are you looking to duplicate the conditions for enabling this CC1 mode
> in IGT, and then checking if the driver set the bit as well?

>From IGT we just request a flip/modeset by setting "max bpc" prop with
different values & check for the dithering status. I found debugfs is the
best way to get this dither status info, since we don't have any KMS prop
to expose this info. Otherwise, we need to read the registers and bitmask
to get the dither status which is hectic. 

- Bhanu

> 
> I thought the direction has been that we don't do this type of
> validation in IGT. There is no end to it.
> 
> Ville?
> 
> > +
> > +	drm_modeset_unlock(&crtc->base.mutex);
> > +
> > +	return 0;
> > +}
> > +DEFINE_SHOW_ATTRIBUTE(dither_state);
> > +
> >  void intel_display_debugfs_register(struct drm_i915_private *i915)
> >  {
> >  	struct drm_minor *minor = i915->drm.primary;
> > +	struct drm_device *dev = &i915->drm;
> > +	struct drm_crtc *crtc;
> >  	int i;
> >
> > +	drm_for_each_crtc(crtc, dev)
> > +		debugfs_create_file("dither", 0444, crtc->debugfs_entry, crtc,
> > +				    &dither_state_fops);
> > +
> 
> See intel_crtc_debugfs_add(), called from intel_crtc_late_register().
> 
> >  	for (i = 0; i < ARRAY_SIZE(intel_display_debugfs_files); i++) {
> >  		debugfs_create_file(intel_display_debugfs_files[i].name,
> >  				    S_IRUGO | S_IWUSR,
> 
> --
> Jani Nikula, Intel Open Source Graphics Center