| Message ID | 1479850766-32748-6-git-send-email-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Nov 22, 2016 at 01:39:26PM -0800, Manasi Navare wrote: > The intel_dp_autotest_video_pattern() function gets invoked through the > compliance test handler on a HPD short pulse if the test type is > set to DP_TEST_VIDEO_PATTERN. This performs the DPCD registers > reads to read the requested test pattern, video pattern resolution, > frame rate and bits per color value. The results of this analysis > are handed off to userspace so that the userspace app can set the > video pattern mode appropriately for the test result/response. > > The compliance_test_active flag is set at the end of the individual > test handling functions. This is so that the kernel-side operations > can be completed without the risk of interruption from the userspace > app that is polling on that flag. > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@intel.com> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 14 +++++++- > drivers/gpu/drm/i915/intel_dp.c | 68 +++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 9 +++++ > 3 files changed, 90 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 3799a12..b048a0e 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -4545,7 +4545,19 @@ static int i915_displayport_test_data_show(struct seq_file *m, void *data) > if (connector->status == connector_status_connected && > connector->encoder != NULL) { > intel_dp = enc_to_intel_dp(connector->encoder); > - seq_printf(m, "%lx", intel_dp->compliance_test_data); > + if (intel_dp->compliance_test_type == > + DP_TEST_LINK_EDID_READ) > + seq_printf(m, "%lx", > + intel_dp->compliance_test_data); > + else if (intel_dp->compliance_test_type == > + DP_TEST_LINK_VIDEO_PATTERN) { > + seq_printf(m, "hdisplay: %d\n", > + intel_dp->test_data.hdisplay); > + seq_printf(m, "vdisplay: %d\n", > + intel_dp->test_data.vdisplay); > + seq_printf(m, "bpc: %u\n", > + intel_dp->test_data.bpc); > + } > } else > seq_puts(m, "0"); > } > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index f93e130..784f86e 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -28,8 +28,10 @@ > #include <linux/i2c.h> > #include <linux/slab.h> > #include <linux/export.h> > +#include <linux/types.h> > #include <linux/notifier.h> > #include <linux/reboot.h> > +#include <asm/byteorder.h> > #include <drm/drmP.h> > #include <drm/drm_atomic_helper.h> > #include <drm/drm_crtc.h> > @@ -3868,7 +3870,73 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp) > static uint8_t intel_dp_autotest_video_pattern(struct intel_dp *intel_dp) > { > uint8_t test_result = DP_TEST_NAK; > + uint8_t test_pattern; > + uint16_t test_misc; > + __be16 h_width, v_height; > + int status = 0; > + > + /* Read the TEST_PATTERN (DP CTS 3.1.5) */ > + status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_PATTERN, > + &test_pattern, 1); > + if (status <= 0) { > + DRM_DEBUG_KMS("Could not read test pattern from " > + "reference sink\n"); > + return 0; > + } > + if (test_pattern != DP_COLOR_RAMP) > + return test_result; > + intel_dp->test_data.video_pattern = test_pattern; > + > + status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_H_WIDTH, > + &h_width, 2); > + if (status <= 0) { > + DRM_DEBUG_KMS("Could not read H Width from " > + "reference sink\n"); > + return 0; > + } > + intel_dp->test_data.hdisplay = be16_to_cpu(h_width); > + > + status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_V_HEIGHT, > + &v_height, 2); > + if (status <= 0) { > + DRM_DEBUG_KMS("Could not read V Height from " > + "reference sink\n"); > + return 0; > + } > + intel_dp->test_data.vdisplay = be16_to_cpu(v_height); > + > + status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_MISC, > + &test_misc, 1); > + if (status <= 0) { > + DRM_DEBUG_KMS("Could not read TEST MISC from " > + "reference sink\n"); > + return 0; > + } > + if (((test_misc & DP_TEST_COLOR_FORMAT_MASK) >> DP_TEST_MISC_BIT_1) != > + DP_VIDEO_PATTERN_RGB_FORMAT) > + return test_result; > + if (((test_misc & DP_TEST_DYNAMIC_RANGE_MASK) >> DP_TEST_MISC_BIT_3) != > + DP_VIDEO_PATTERN_VESA) > + return test_result; > + switch ((test_misc & DP_TEST_BIT_DEPTH_MASK) >> DP_TEST_MISC_BIT_5) { > + case 0: > + intel_dp->compliance_force_bpc = 6; > + intel_dp->test_data.bpc = 6; > + break; > + case 1: > + intel_dp->compliance_force_bpc = 8; > + intel_dp->test_data.bpc = 8; > + break; > + default: > + return test_result; > + } > + /* Set test active flag here so userspace doesn't interrupt things */ > + intel_dp->compliance_test_active = 1; > + > + test_result = DP_TEST_ACK; > + > return test_result; > + > } > > static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 3eb428e..ff4431e 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -880,6 +880,12 @@ struct intel_dp_desc { > u8 sw_minor_rev; > } __packed; > > +struct compliance_test_data { > + uint8_t video_pattern; > + uint16_t hdisplay, vdisplay; > + uint8_t bpc; > +}; > + > struct intel_dp { > i915_reg_t output_reg; > i915_reg_t aux_ch_ctl_reg; > @@ -961,6 +967,9 @@ struct intel_dp { > u8 compliance_test_lane_count; > u8 compliance_test_link_rate; > unsigned long compliance_force_bpc; /* 0 for default or bpc to use */ > + struct compliance_test_data test_data; /* a structure to hold all dp > + * compliance test data > + */ I like this idea of tying all the compliance test data into a sub-struct, but please refactor the existing code (and the force_bpc you're adding in this series) to also use that. Consistency wins over pretty imo. Otherwise seems all reasonable, but this isn't a detailed review. -Daniel
On Tue, 22 Nov 2016, Manasi Navare <manasi.d.navare@intel.com> wrote: > The intel_dp_autotest_video_pattern() function gets invoked through the > compliance test handler on a HPD short pulse if the test type is > set to DP_TEST_VIDEO_PATTERN. This performs the DPCD registers > reads to read the requested test pattern, video pattern resolution, > frame rate and bits per color value. The results of this analysis > are handed off to userspace so that the userspace app can set the > video pattern mode appropriately for the test result/response. > > The compliance_test_active flag is set at the end of the individual > test handling functions. This is so that the kernel-side operations > can be completed without the risk of interruption from the userspace > app that is polling on that flag. I've brought this up before, but I think for this stuff the way to go is to have the userspace read the DPCD directly. We have the dev node for it. With the approach in this patch, we'll just end up reading a bunch of stuff from DPCD in kernel, doing error handling for that, decoding and sanity checking the values, putting them in debugfs for the userspace to read, having userspace code read debugfs, doing error handling for that, decoding and sanity checking the data, finally doing something based on the data. You'll also get a *much* faster turnaround for getting your userspace code done than getting all of this in kernel first, then tweaking your userspace, having to update both of those in lockstep, etc. When this is based on reading DPCD directly, you can just add new stuff quickly in userspace, with no kernel dependency. The easiest way would be to have an indication in debugfs for userspace that there's something interesting in DPCD. Just a simple thing. BR, Jani. > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@intel.com> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 14 +++++++- > drivers/gpu/drm/i915/intel_dp.c | 68 +++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 9 +++++ > 3 files changed, 90 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 3799a12..b048a0e 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -4545,7 +4545,19 @@ static int i915_displayport_test_data_show(struct seq_file *m, void *data) > if (connector->status == connector_status_connected && > connector->encoder != NULL) { > intel_dp = enc_to_intel_dp(connector->encoder); > - seq_printf(m, "%lx", intel_dp->compliance_test_data); > + if (intel_dp->compliance_test_type == > + DP_TEST_LINK_EDID_READ) > + seq_printf(m, "%lx", > + intel_dp->compliance_test_data); > + else if (intel_dp->compliance_test_type == > + DP_TEST_LINK_VIDEO_PATTERN) { > + seq_printf(m, "hdisplay: %d\n", > + intel_dp->test_data.hdisplay); > + seq_printf(m, "vdisplay: %d\n", > + intel_dp->test_data.vdisplay); > + seq_printf(m, "bpc: %u\n", > + intel_dp->test_data.bpc); > + } > } else > seq_puts(m, "0"); > } > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index f93e130..784f86e 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -28,8 +28,10 @@ > #include <linux/i2c.h> > #include <linux/slab.h> > #include <linux/export.h> > +#include <linux/types.h> > #include <linux/notifier.h> > #include <linux/reboot.h> > +#include <asm/byteorder.h> > #include <drm/drmP.h> > #include <drm/drm_atomic_helper.h> > #include <drm/drm_crtc.h> > @@ -3868,7 +3870,73 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp) > static uint8_t intel_dp_autotest_video_pattern(struct intel_dp *intel_dp) > { > uint8_t test_result = DP_TEST_NAK; > + uint8_t test_pattern; > + uint16_t test_misc; > + __be16 h_width, v_height; > + int status = 0; > + > + /* Read the TEST_PATTERN (DP CTS 3.1.5) */ > + status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_PATTERN, > + &test_pattern, 1); > + if (status <= 0) { > + DRM_DEBUG_KMS("Could not read test pattern from " > + "reference sink\n"); > + return 0; > + } > + if (test_pattern != DP_COLOR_RAMP) > + return test_result; > + intel_dp->test_data.video_pattern = test_pattern; > + > + status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_H_WIDTH, > + &h_width, 2); > + if (status <= 0) { > + DRM_DEBUG_KMS("Could not read H Width from " > + "reference sink\n"); > + return 0; > + } > + intel_dp->test_data.hdisplay = be16_to_cpu(h_width); > + > + status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_V_HEIGHT, > + &v_height, 2); > + if (status <= 0) { > + DRM_DEBUG_KMS("Could not read V Height from " > + "reference sink\n"); > + return 0; > + } > + intel_dp->test_data.vdisplay = be16_to_cpu(v_height); > + > + status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_MISC, > + &test_misc, 1); > + if (status <= 0) { > + DRM_DEBUG_KMS("Could not read TEST MISC from " > + "reference sink\n"); > + return 0; > + } > + if (((test_misc & DP_TEST_COLOR_FORMAT_MASK) >> DP_TEST_MISC_BIT_1) != > + DP_VIDEO_PATTERN_RGB_FORMAT) > + return test_result; > + if (((test_misc & DP_TEST_DYNAMIC_RANGE_MASK) >> DP_TEST_MISC_BIT_3) != > + DP_VIDEO_PATTERN_VESA) > + return test_result; > + switch ((test_misc & DP_TEST_BIT_DEPTH_MASK) >> DP_TEST_MISC_BIT_5) { > + case 0: > + intel_dp->compliance_force_bpc = 6; > + intel_dp->test_data.bpc = 6; > + break; > + case 1: > + intel_dp->compliance_force_bpc = 8; > + intel_dp->test_data.bpc = 8; > + break; > + default: > + return test_result; > + } > + /* Set test active flag here so userspace doesn't interrupt things */ > + intel_dp->compliance_test_active = 1; > + > + test_result = DP_TEST_ACK; > + > return test_result; > + > } > > static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 3eb428e..ff4431e 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -880,6 +880,12 @@ struct intel_dp_desc { > u8 sw_minor_rev; > } __packed; > > +struct compliance_test_data { > + uint8_t video_pattern; > + uint16_t hdisplay, vdisplay; > + uint8_t bpc; > +}; > + > struct intel_dp { > i915_reg_t output_reg; > i915_reg_t aux_ch_ctl_reg; > @@ -961,6 +967,9 @@ struct intel_dp { > u8 compliance_test_lane_count; > u8 compliance_test_link_rate; > unsigned long compliance_force_bpc; /* 0 for default or bpc to use */ > + struct compliance_test_data test_data; /* a structure to hold all dp > + * compliance test data > + */ > }; > > struct intel_lspcon {
On Wed, Nov 23, 2016 at 03:37:24PM +0200, Jani Nikula wrote: > On Tue, 22 Nov 2016, Manasi Navare <manasi.d.navare@intel.com> wrote: > > The intel_dp_autotest_video_pattern() function gets invoked through the > > compliance test handler on a HPD short pulse if the test type is > > set to DP_TEST_VIDEO_PATTERN. This performs the DPCD registers > > reads to read the requested test pattern, video pattern resolution, > > frame rate and bits per color value. The results of this analysis > > are handed off to userspace so that the userspace app can set the > > video pattern mode appropriately for the test result/response. > > > > The compliance_test_active flag is set at the end of the individual > > test handling functions. This is so that the kernel-side operations > > can be completed without the risk of interruption from the userspace > > app that is polling on that flag. > > I've brought this up before, but I think for this stuff the way to go is > to have the userspace read the DPCD directly. We have the dev node for > it. > > With the approach in this patch, we'll just end up reading a bunch of > stuff from DPCD in kernel, doing error handling for that, decoding and > sanity checking the values, putting them in debugfs for the userspace to > read, having userspace code read debugfs, doing error handling for that, > decoding and sanity checking the data, finally doing something based on > the data. > > You'll also get a *much* faster turnaround for getting your userspace > code done than getting all of this in kernel first, then tweaking your > userspace, having to update both of those in lockstep, etc. When this is > based on reading DPCD directly, you can just add new stuff quickly in > userspace, with no kernel dependency. > > The easiest way would be to have an indication in debugfs for userspace > that there's something interesting in DPCD. Just a simple thing. Or just have the kernel fire off an uevent...
On Wed, 23 Nov 2016, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Wed, Nov 23, 2016 at 03:37:24PM +0200, Jani Nikula wrote: >> On Tue, 22 Nov 2016, Manasi Navare <manasi.d.navare@intel.com> wrote: >> > The intel_dp_autotest_video_pattern() function gets invoked through the >> > compliance test handler on a HPD short pulse if the test type is >> > set to DP_TEST_VIDEO_PATTERN. This performs the DPCD registers >> > reads to read the requested test pattern, video pattern resolution, >> > frame rate and bits per color value. The results of this analysis >> > are handed off to userspace so that the userspace app can set the >> > video pattern mode appropriately for the test result/response. >> > >> > The compliance_test_active flag is set at the end of the individual >> > test handling functions. This is so that the kernel-side operations >> > can be completed without the risk of interruption from the userspace >> > app that is polling on that flag. >> >> I've brought this up before, but I think for this stuff the way to go is >> to have the userspace read the DPCD directly. We have the dev node for >> it. >> >> With the approach in this patch, we'll just end up reading a bunch of >> stuff from DPCD in kernel, doing error handling for that, decoding and >> sanity checking the values, putting them in debugfs for the userspace to >> read, having userspace code read debugfs, doing error handling for that, >> decoding and sanity checking the data, finally doing something based on >> the data. >> >> You'll also get a *much* faster turnaround for getting your userspace >> code done than getting all of this in kernel first, then tweaking your >> userspace, having to update both of those in lockstep, etc. When this is >> based on reading DPCD directly, you can just add new stuff quickly in >> userspace, with no kernel dependency. >> >> The easiest way would be to have an indication in debugfs for userspace >> that there's something interesting in DPCD. Just a simple thing. > > Or just have the kernel fire off an uevent... I know, I know, I had that written in my mail already, but swallowed it not to add another dependency on another new UABI... we can do that later. BR, Jani.
On Wed, Nov 23, 2016 at 03:58:34PM +0200, Jani Nikula wrote: > On Wed, 23 Nov 2016, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > On Wed, Nov 23, 2016 at 03:37:24PM +0200, Jani Nikula wrote: > >> On Tue, 22 Nov 2016, Manasi Navare <manasi.d.navare@intel.com> wrote: > >> > The intel_dp_autotest_video_pattern() function gets invoked through the > >> > compliance test handler on a HPD short pulse if the test type is > >> > set to DP_TEST_VIDEO_PATTERN. This performs the DPCD registers > >> > reads to read the requested test pattern, video pattern resolution, > >> > frame rate and bits per color value. The results of this analysis > >> > are handed off to userspace so that the userspace app can set the > >> > video pattern mode appropriately for the test result/response. > >> > > >> > The compliance_test_active flag is set at the end of the individual > >> > test handling functions. This is so that the kernel-side operations > >> > can be completed without the risk of interruption from the userspace > >> > app that is polling on that flag. > >> > >> I've brought this up before, but I think for this stuff the way to go is > >> to have the userspace read the DPCD directly. We have the dev node for > >> it. > >> > >> With the approach in this patch, we'll just end up reading a bunch of > >> stuff from DPCD in kernel, doing error handling for that, decoding and > >> sanity checking the values, putting them in debugfs for the userspace to > >> read, having userspace code read debugfs, doing error handling for that, > >> decoding and sanity checking the data, finally doing something based on > >> the data. > >> > >> You'll also get a *much* faster turnaround for getting your userspace > >> code done than getting all of this in kernel first, then tweaking your > >> userspace, having to update both of those in lockstep, etc. When this is > >> based on reading DPCD directly, you can just add new stuff quickly in > >> userspace, with no kernel dependency. > >> > >> The easiest way would be to have an indication in debugfs for userspace > >> that there's something interesting in DPCD. Just a simple thing. > > > > Or just have the kernel fire off an uevent... > > I know, I know, I had that written in my mail already, but swallowed it > not to add another dependency on another new UABI... we can do that > later. In principle I'd agree, but the trouble with dp compliance is that the CTS expects us to do stupid shit sometimes, like that test pattern forced as 6bpc. So if we decode&handle everything in userspace then the question is how to feed all those special cases back into the kernel, which means more debugfs interfaces. Just going in the other direction. Given that it's a bit a toss-up unfortunately, and the current approach seems reasonable. -Daniel
On Wed, Nov 23, 2016 at 03:17:53PM +0100, Daniel Vetter wrote: > On Wed, Nov 23, 2016 at 03:58:34PM +0200, Jani Nikula wrote: > > On Wed, 23 Nov 2016, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > > On Wed, Nov 23, 2016 at 03:37:24PM +0200, Jani Nikula wrote: > > >> On Tue, 22 Nov 2016, Manasi Navare <manasi.d.navare@intel.com> wrote: > > >> > The intel_dp_autotest_video_pattern() function gets invoked through the > > >> > compliance test handler on a HPD short pulse if the test type is > > >> > set to DP_TEST_VIDEO_PATTERN. This performs the DPCD registers > > >> > reads to read the requested test pattern, video pattern resolution, > > >> > frame rate and bits per color value. The results of this analysis > > >> > are handed off to userspace so that the userspace app can set the > > >> > video pattern mode appropriately for the test result/response. > > >> > > > >> > The compliance_test_active flag is set at the end of the individual > > >> > test handling functions. This is so that the kernel-side operations > > >> > can be completed without the risk of interruption from the userspace > > >> > app that is polling on that flag. > > >> > > >> I've brought this up before, but I think for this stuff the way to go is > > >> to have the userspace read the DPCD directly. We have the dev node for > > >> it. > > >> > > >> With the approach in this patch, we'll just end up reading a bunch of > > >> stuff from DPCD in kernel, doing error handling for that, decoding and > > >> sanity checking the values, putting them in debugfs for the userspace to > > >> read, having userspace code read debugfs, doing error handling for that, > > >> decoding and sanity checking the data, finally doing something based on > > >> the data. > > >> > > >> You'll also get a *much* faster turnaround for getting your userspace > > >> code done than getting all of this in kernel first, then tweaking your > > >> userspace, having to update both of those in lockstep, etc. When this is > > >> based on reading DPCD directly, you can just add new stuff quickly in > > >> userspace, with no kernel dependency. > > >> > > >> The easiest way would be to have an indication in debugfs for userspace > > >> that there's something interesting in DPCD. Just a simple thing. > > > > > > Or just have the kernel fire off an uevent... > > > > I know, I know, I had that written in my mail already, but swallowed it > > not to add another dependency on another new UABI... we can do that > > later. > > In principle I'd agree, but the trouble with dp compliance is that the CTS > expects us to do stupid shit sometimes, like that test pattern forced as > 6bpc. So if we decode&handle everything in userspace then the question is > how to feed all those special cases back into the kernel, which means more > debugfs interfaces. Just going in the other direction. > > Given that it's a bit a toss-up unfortunately, and the current approach > seems reasonable. The kernel could of course parse whatever is needed from the test request as well, but leave it up to userspace to initiate the actual test and ack/nack the test request. So I don't see a real need for a debugfs interface in either direction.
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 3799a12..b048a0e 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -4545,7 +4545,19 @@ static int i915_displayport_test_data_show(struct seq_file *m, void *data) if (connector->status == connector_status_connected && connector->encoder != NULL) { intel_dp = enc_to_intel_dp(connector->encoder); - seq_printf(m, "%lx", intel_dp->compliance_test_data); + if (intel_dp->compliance_test_type == + DP_TEST_LINK_EDID_READ) + seq_printf(m, "%lx", + intel_dp->compliance_test_data); + else if (intel_dp->compliance_test_type == + DP_TEST_LINK_VIDEO_PATTERN) { + seq_printf(m, "hdisplay: %d\n", + intel_dp->test_data.hdisplay); + seq_printf(m, "vdisplay: %d\n", + intel_dp->test_data.vdisplay); + seq_printf(m, "bpc: %u\n", + intel_dp->test_data.bpc); + } } else seq_puts(m, "0"); } diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index f93e130..784f86e 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -28,8 +28,10 @@ #include <linux/i2c.h> #include <linux/slab.h> #include <linux/export.h> +#include <linux/types.h> #include <linux/notifier.h> #include <linux/reboot.h> +#include <asm/byteorder.h> #include <drm/drmP.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_crtc.h> @@ -3868,7 +3870,73 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp) static uint8_t intel_dp_autotest_video_pattern(struct intel_dp *intel_dp) { uint8_t test_result = DP_TEST_NAK; + uint8_t test_pattern; + uint16_t test_misc; + __be16 h_width, v_height; + int status = 0; + + /* Read the TEST_PATTERN (DP CTS 3.1.5) */ + status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_PATTERN, + &test_pattern, 1); + if (status <= 0) { + DRM_DEBUG_KMS("Could not read test pattern from " + "reference sink\n"); + return 0; + } + if (test_pattern != DP_COLOR_RAMP) + return test_result; + intel_dp->test_data.video_pattern = test_pattern; + + status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_H_WIDTH, + &h_width, 2); + if (status <= 0) { + DRM_DEBUG_KMS("Could not read H Width from " + "reference sink\n"); + return 0; + } + intel_dp->test_data.hdisplay = be16_to_cpu(h_width); + + status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_V_HEIGHT, + &v_height, 2); + if (status <= 0) { + DRM_DEBUG_KMS("Could not read V Height from " + "reference sink\n"); + return 0; + } + intel_dp->test_data.vdisplay = be16_to_cpu(v_height); + + status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_MISC, + &test_misc, 1); + if (status <= 0) { + DRM_DEBUG_KMS("Could not read TEST MISC from " + "reference sink\n"); + return 0; + } + if (((test_misc & DP_TEST_COLOR_FORMAT_MASK) >> DP_TEST_MISC_BIT_1) != + DP_VIDEO_PATTERN_RGB_FORMAT) + return test_result; + if (((test_misc & DP_TEST_DYNAMIC_RANGE_MASK) >> DP_TEST_MISC_BIT_3) != + DP_VIDEO_PATTERN_VESA) + return test_result; + switch ((test_misc & DP_TEST_BIT_DEPTH_MASK) >> DP_TEST_MISC_BIT_5) { + case 0: + intel_dp->compliance_force_bpc = 6; + intel_dp->test_data.bpc = 6; + break; + case 1: + intel_dp->compliance_force_bpc = 8; + intel_dp->test_data.bpc = 8; + break; + default: + return test_result; + } + /* Set test active flag here so userspace doesn't interrupt things */ + intel_dp->compliance_test_active = 1; + + test_result = DP_TEST_ACK; + return test_result; + } static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 3eb428e..ff4431e 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -880,6 +880,12 @@ struct intel_dp_desc { u8 sw_minor_rev; } __packed; +struct compliance_test_data { + uint8_t video_pattern; + uint16_t hdisplay, vdisplay; + uint8_t bpc; +}; + struct intel_dp { i915_reg_t output_reg; i915_reg_t aux_ch_ctl_reg; @@ -961,6 +967,9 @@ struct intel_dp { u8 compliance_test_lane_count; u8 compliance_test_link_rate; unsigned long compliance_force_bpc; /* 0 for default or bpc to use */ + struct compliance_test_data test_data; /* a structure to hold all dp + * compliance test data + */ }; struct intel_lspcon {
The intel_dp_autotest_video_pattern() function gets invoked through the compliance test handler on a HPD short pulse if the test type is set to DP_TEST_VIDEO_PATTERN. This performs the DPCD registers reads to read the requested test pattern, video pattern resolution, frame rate and bits per color value. The results of this analysis are handed off to userspace so that the userspace app can set the video pattern mode appropriately for the test result/response. The compliance_test_active flag is set at the end of the individual test handling functions. This is so that the kernel-side operations can be completed without the risk of interruption from the userspace app that is polling on that flag. Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@intel.com> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 14 +++++++- drivers/gpu/drm/i915/intel_dp.c | 68 +++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_drv.h | 9 +++++ 3 files changed, 90 insertions(+), 1 deletion(-)