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

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

[Rodrigo Vivi]

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.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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;
 		}
 
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Show debugfs dpcd read failure directly

[Patchwork]

== Series Details ==

Series: drm/i915: Show debugfs dpcd read failure directly
URL   : https://patchwork.freedesktop.org/series/46977/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4521 -> Patchwork_9737 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/46977/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9737 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_hangcheck:
      fi-kbl-7500u:       PASS -> DMESG-FAIL (fdo#106947, fdo#106560)

    igt@kms_chamelium@hdmi-hpd-fast:
      fi-kbl-7500u:       SKIP -> FAIL (fdo#102672, fdo#103841)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         PASS -> INCOMPLETE (fdo#103927)

    
    ==== Possible fixes ====

    igt@debugfs_test@read_all_entries:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         FAIL (fdo#104008) -> PASS

    
  fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947


== Participating hosts (47 -> 42) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4521 -> Patchwork_9737

  CI_DRM_4521: a4ebbd84c682fd30edbde6ac0e48d150d4c5c066 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4570: 65cdccdc7bcbb791d791aeeeecb784a382110a3c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9737: 84fb667b4f2eaba40302eaa18d7ca1297008d07c @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

84fb667b4f2e drm/i915: Show debugfs dpcd read failure directly

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9737/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Show debugfs dpcd read failure directly

[Dhinakaran Pandiyan]

On Fri, 2018-07-20 at 10:41 -0700, Rodrigo Vivi 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,

We can print partial ( and successful) output and return the first
error value we get.

We should probably get rid of that warn_on too.

>  but we shouldn't
> need to check 2 places to see why reg is not on the list.
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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;
>  		}
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915: Show debugfs dpcd read failure directly

[Patchwork]

== Series Details ==

Series: drm/i915: Show debugfs dpcd read failure directly
URL   : https://patchwork.freedesktop.org/series/46977/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4521_full -> Patchwork_9737_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9737_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9737_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9737_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-blt:
      shard-kbl:          SKIP -> PASS

    igt@gem_exec_schedule@deep-render:
      shard-kbl:          PASS -> SKIP

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
      shard-snb:          PASS -> SKIP +1

    
== Known issues ==

  Here are the changes found in Patchwork_9737_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665, fdo#106023)

    igt@kms_cursor_legacy@cursor-vs-flip-toggle:
      shard-hsw:          PASS -> FAIL (fdo#103355)

    igt@kms_flip@2x-flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#102887)

    igt@kms_rotation_crc@primary-rotation-90:
      shard-apl:          PASS -> FAIL (fdo#103925)

    
    ==== Possible fixes ====

    igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
      shard-hsw:          FAIL (fdo#105767) -> PASS

    igt@kms_flip@dpms-vs-vblank-race:
      shard-glk:          FAIL (fdo#103060) -> PASS

    
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#105767 https://bugs.freedesktop.org/show_bug.cgi?id=105767
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4521 -> Patchwork_9737

  CI_DRM_4521: a4ebbd84c682fd30edbde6ac0e48d150d4c5c066 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4570: 65cdccdc7bcbb791d791aeeeecb784a382110a3c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9737: 84fb667b4f2eaba40302eaa18d7ca1297008d07c @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9737/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Show debugfs dpcd read failure directly

[Chris Wilson]

Quoting Rodrigo Vivi (2018-07-20 18:41:10)
> 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.
 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106371
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Show debugfs dpcd read failure directly

[Jani Nikula]

On Fri, 20 Jul 2018, Rodrigo Vivi <rodrigo.vivi@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?

BR,
Jani.

>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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;
>  		}

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Show debugfs dpcd read failure directly

[Pandiyan, Dhinakaran]

On Mon, 2018-08-20 at 10:15 +0300, Jani Nikula wrote:
> On Fri, 20 Jul 2018, Rodrigo Vivi <rodrigo.vivi@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.

-DK


> 
> BR,
> Jani.
> 
> > 
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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;
> >  		}
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [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@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@intel.com>
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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;
> > >  		}
> > 
> > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx