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 <[email protected]>
> 
> 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 <[email protected]>
> 
> 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
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to