[Intel-gfx] [RFC 4/7] drm/i915/dp: Notify testapp using uevent and debugfs entry

[Manasi Navare]

On Sun, Nov 17, 2019 at 08:58:45PM -0800, Manasi Navare wrote:
> On Fri, Nov 15, 2019 at 08:55:46PM +0530, Animesh Manna wrote:
> > To align with link compliance design existing intel_dp_compliance
> > tool will be used to get the phy request in userspace through uevent.
> > 
> > Signed-off-by: Animesh Manna <animesh.manna at intel.com>
> 
> I would prefer splitting this patch since sending a uevent is more related
> to the PHY test prep handling and debugfs handling can all be in a separate
> patch.
> I prefer this because debugfs nodes might need to change in the future based
> on more requirements or testing feedback so its better for that to be in separate
> patch.
> 
> you could add the hotplug event sending part to the prep patch (3/7) and mention that
> in the commit message
> 
> Debugfs part looks good to me. Have you tested the debugfs nodes and validated if this
> information is being written in the correct form?
> 
> After the split and validation of debugs nodes:
> 
> Acked-by: Manasi Navare <manasi.d.navare at intel.com>
> 
> Manasi
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c |  6 ++++--
> >  drivers/gpu/drm/i915/i915_debugfs.c     | 10 ++++++++++
> >  2 files changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 338d3744c5d5..a2b860cf3b93 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -5288,8 +5288,10 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
> >  
> >  	intel_psr_short_pulse(intel_dp);
> >  
> > -	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
> > -		DRM_DEBUG_KMS("Link Training Compliance Test requested\n");
> > +	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING ||
> > +	    intel_dp->compliance.test_type ==
> > +	    DP_TEST_LINK_PHY_TEST_PATTERN) {
> > +		DRM_DEBUG_KMS("Compliance Test requested\n");

One more change I think here would be good IMO for debugging is that
you should print test_type in DEBUG_KMS

Manasi

> >  		/* Send a Hotplug Uevent to userspace to start modeset */
> >  		drm_kms_helper_hotplug_event(&dev_priv->drm);
> >  	}
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index cab632791f73..e8b1a8c1015a 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -3212,6 +3212,16 @@ static int i915_displayport_test_data_show(struct seq_file *m, void *data)
> >  					   intel_dp->compliance.test_data.vdisplay);
> >  				seq_printf(m, "bpc: %u\n",
> >  					   intel_dp->compliance.test_data.bpc);
> > +			} else if (intel_dp->compliance.test_type ==
> > +				   DP_TEST_LINK_PHY_TEST_PATTERN) {
> > +				seq_printf(m, "pattern: %d\n",
> > +					   intel_dp->compliance.test_data.phytest.phy_pattern);
> > +				seq_printf(m, "Number of lanes: %d\n",
> > +					   intel_dp->compliance.test_data.phytest.num_lanes);
> > +				seq_printf(m, "Link Rate: %d\n",
> > +					   intel_dp->compliance.test_data.phytest.link_rate);
> > +				seq_printf(m, "level: %02x\n",
> > +					   intel_dp->train_set[0]);
> >  			}
> >  		} else
> >  			seq_puts(m, "0");
> > -- 
> > 2.22.0
> > 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx