| Message ID | 20191115152549.23047-5-animesh.manna@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DP Phy compliace auto test | expand |
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@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@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"); > /* 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 >
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@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@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 11/18/2019 10:36 AM, Manasi Navare wrote: > 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@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 Sure. >> >> Debugfs part looks good to me. Have you tested the debugfs nodes and validated if this >> information is being written in the correct form? Tested the test-type entry, planning to add a newline char, otherwise working. >> >> After the split and validation of debugs nodes: >> >> Acked-by: Manasi Navare <manasi.d.navare@intel.com> Thanks. >> >> 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 Sure. Regards, Animesh > > 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@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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"); /* 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");
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@intel.com> --- drivers/gpu/drm/i915/display/intel_dp.c | 6 ++++-- drivers/gpu/drm/i915/i915_debugfs.c | 10 ++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-)