On Sat, 02 Nov 2013, Todd Previte <tprevite@xxxxxxxxx> 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 <tprevite@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
