From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH] drm/i915: Implement Displayport automated testing Date: Tue, 05 Nov 2013 11:21:46 +0200 Message-ID: <8738nbryol.fsf@intel.com> References: <1380882730-32207-1-git-send-email-tprevite@gmail.com> <1383345848-6872-1-git-send-email-tprevite@gmail.com> <1383345848-6872-2-git-send-email-tprevite@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id 0BE07FB05E for ; Tue, 5 Nov 2013 01:19:52 -0800 (PST) In-Reply-To: <1383345848-6872-2-git-send-email-tprevite@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Todd Previte , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Sat, 02 Nov 2013, Todd Previte wrote: > This initial patch adds support for automated testing of the source device > to the i915 driver. Most of this patch is infrastructure for the tests; > follow up patches will add support for the individual tests with updates > to ACK the tests that are supported (or NAK if the test > fails/is unsupported). > > Signed-off-by: Todd Previte > --- > drivers/gpu/drm/i915/intel_dp.c | 87 +++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 84 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index c8515bb..5f2a720 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -2813,11 +2813,92 @@ intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector) > return true; > } > > +/* Automated test function hook - description forthcoming */ > +static bool > +intel_dp_autotest_link_training(struct intel_dp *intel_dp) > +{ > + bool test_result = false; > + return test_result; > +} > + > +/* Automated test function hook - description forthcoming */ > +static bool > +intel_dp_autotest_video_pattern(struct intel_dp *intel_dp) > +{ > + bool test_result = false; > + return test_result; > +} > + > +/* Automated test function hook - description forthcoming */ > +static bool > +intel_dp_autotest_edid(struct intel_dp *intel_dp) > +{ > + bool test_result = false; > + return test_result; > +} > + > +/* Automated test function hook - description forthcoming */ > +static bool > +intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp) > +{ > + bool test_result = false; > + return test_result; > +} > + > +/* Automated test function hook - description forthcoming */ > +static bool > +intel_dp_autotest_faux_pattern(struct intel_dp *intel_dp) > +{ > + bool test_result = false; > + return test_result; > +} > + > static void > intel_dp_handle_test_request(struct intel_dp *intel_dp) > -{ > - /* NAK by default */ > - intel_dp_aux_native_write_1(intel_dp, DP_TEST_RESPONSE, DP_TEST_NAK); > +{ > + uint8_t response = DP_TEST_NAK; > + bool result = false; > + uint8_t rxdata = 0; > + > + DRM_DEBUG_KMS("Displayport: Recvd automated test request\n"); > + /* Read DP_TEST_REQUEST register to identify the requested test */ > + intel_dp_aux_native_read_retry(intel_dp, DP_TEST_REQUEST, &rxdata, 1); I don't think you need _retry here. It's only needed for the case when this might wake up the sink, but the call is cargo culted all over the place... > + /* Determine which test has been requested */ I think we get that without the comment. ;) Is it possible multiple test request bits are set at the same time, or will there always be just one? The spec is a bit unclear on this. > + switch (rxdata) { > + /* ACK/NAK response based on the success or failure of the specified > + automated test function. Unimplemented tests will NAK as will those > + that are unsupported. */ > + case DP_TEST_LINK_TRAINING: > + DRM_DEBUG_KMS("Displayport: Executing LINK_TRAINING request\n"); > + result = intel_dp_autotest_link_training(intel_dp); > + break; > + case DP_TEST_LINK_VIDEO_PATTERN: > + DRM_DEBUG_KMS("Displayport: Executing TEST_PATTERN request\n"); > + result = intel_dp_autotest_video_pattern(intel_dp); > + break; > + case DP_TEST_LINK_EDID_READ: > + DRM_DEBUG_KMS("Displayport: Executing EDID request\n"); > + result = intel_dp_autotest_edid(intel_dp); > + break; > + case DP_TEST_LINK_PHY_TEST_PATTERN: > + DRM_DEBUG_KMS("Displayport: Executing PHY_PATTERN request\n"); > + result = intel_dp_autotest_phy_pattern(intel_dp); > + break; > + case DP_TEST_LINK_FAUX_PATTERN: > + printk(KERN_DEBUG "Displayport: Executing FAUX_PATTERN request \n"); > + result = intel_dp_autotest_faux_pattern(intel_dp); > + break; Use tabs to indent. > + /* Unsupported test case or something went wrong */ > + default: > + /* Log error here for unhandled test request */ > + DRM_DEBUG_KMS("Displayport: Error - unhandled automated test type\n"); > + break; > + } > + /* Check for a valid test execution */ > + if (result == true) if (result) is customary. An alternative would be to have the test calls return the response to be written to DP_TEST_RESPONSE, but I'm fine either way. BR, Jani. > + response = DP_TEST_ACK; > + /* Send ACK/NAK based on action taken above */ > + intel_dp_aux_native_write_1(intel_dp, DP_TEST_RESPONSE, response); > } > > /* > -- > 1.8.1.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center