[Intel-gfx] [PATCH] drm/i915: Show debugfs dpcd read failure directly

[Rodrigo Vivi]

On Mon, Aug 20, 2018 at 12:10:08PM -0700, Pandiyan, Dhinakaran wrote:
> 
> 
> On Mon, 2018-08-20 at 10:15 +0300, Jani Nikula wrote:
> > On Fri, 20 Jul 2018, Rodrigo Vivi <rodrigo.vivi at intel.com> wrote:
> > > Instead of using a backchannel if some dpcd read failed we
> > > can show that directly on debugfs output.
> > > 
> > > We are not returning an error because we might still want
> > > to know if sub-sequent reads works, but we shouldn't
> > > need to check 2 places to see why reg is not on the list.
> > 
> > Should we just nuke this debugfs and use the aux chardev interface to
> > dpcd instead?
> 
> Given that this debugfs does not print decoded information, I think, it
> offers very little benefit over direct reads. We also print some DPCD's
> in dmesg, so I'm in favour of killing it. Don't see the file being used
> in IGTs either.
> 
> Your comment also reminds about the IGT tool that Tarun started writing
> for reading DPCD.

There might be people out there using this for debug. So we should first
be able to do this in some place like IGT:
https://patchwork.freedesktop.org/series/48470/ ?

then we remove. But that discussion might take a while. So I'd like
to move with this fix here now and remove dpcd_show completely when
we create some infrastructure on IGT.

> 
> -DK
> 
> 
> > 
> > BR,
> > Jani.
> > 
> > > 
> > > Cc: Jani Nikula <jani.nikula at intel.com>
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index 59dc0610ea44..5d8da4e8c444 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -4846,8 +4846,8 @@ static int i915_dpcd_show(struct seq_file *m,
> > > void *data)
> > >  
> > >  		err = drm_dp_dpcd_read(&intel_dp->aux, b->offset,
> > > buf, size);
> > >  		if (err <= 0) {
> > > -			DRM_ERROR("dpcd read (%zu bytes at %u)
> > > failed (%zd)\n",
> > > -				  size, b->offset, err);
> > > +			seq_printf(m, "dpcd read (%zu bytes at %u)
> > > failed (%zd)\n",
> > > +				   size, b->offset, err);
> > >  			continue;
> > >  		}
> > 
> >