From mboxrd@z Thu Jan 1 00:00:00 1970 From: Todd Previte Subject: Re: [PATCH] drm/i915: Implement Displayport automated testing Date: Fri, 04 Oct 2013 16:00:41 -0700 Message-ID: <524F4899.9040009@gmail.com> References: <1380882730-32207-1-git-send-email-tprevite@gmail.com> <20131004104514.GD31587@nuc-i3427.alporthouse.com> <524F04D4.2030002@gmail.com> <20131004203925.GA26483@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-pb0-f46.google.com (mail-pb0-f46.google.com [209.85.160.46]) by gabe.freedesktop.org (Postfix) with ESMTP id 8AA70E805D for ; Fri, 4 Oct 2013 15:58:58 -0700 (PDT) Received: by mail-pb0-f46.google.com with SMTP id rq2so4608109pbb.33 for ; Fri, 04 Oct 2013 15:58:58 -0700 (PDT) In-Reply-To: <20131004203925.GA26483@bwidawsk.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Ben Widawsky Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On 10/4/13 1:39 PM, Ben Widawsky wrote: > On Fri, Oct 04, 2013 at 11:11:32AM -0700, Todd Previte wrote: >> On 10/4/13 3:45 AM, Chris Wilson wrote: >>> On Fri, Oct 04, 2013 at 03:32:10AM -0700, 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 | 108 +++++++++++++++++++++++++++++++++++++++- >>>> include/drm/drm_dp_helper.h | 3 +- >>>> 2 files changed, 108 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >>>> index 9770160..a042d59 100644 >>>> --- a/drivers/gpu/drm/i915/intel_dp.c >>>> +++ b/drivers/gpu/drm/i915/intel_dp.c >>>> @@ -64,6 +64,26 @@ static const struct dp_link_dpll vlv_dpll[] = { >>>> { .p1 = 2, .p2 = 2, .n = 1, .m1 = 2, .m2 = 27 } } >>>> }; >>>> >>>> +/****************************************************************************** >>>> +******** Displayport automated testing ******** >>>> +******************************************************************************/ >>>> +/* Automated testing function - link training */ >>>> +static bool >>>> +intel_dp_autotest_link_training(struct intel_dp *intel_dp); >>> The function comment does little more than spell out the function name. >>> What I would prefer to see is a theory-of-operation in the block >>> comment. And all these forward declarations can disappear with clear >>> ordering in the source code. >>> -Chris >>> >> I wasn't sure which way to go with this and opted for the >> declarations, but it's just as easy to reorder the definitions to be >> prior to their use in the handler. I'll get that fixed up and add >> more useful comments above the functions for V2. Thanks Chris. >> >> >> -T > Since it sounds like you have a v2 in the works anyway, can you please > change printk to DRM_DEBUG_KMS. > I have another patch that's following onto this one once the drm header changes get through, so I can either put this in that patch or change them in a separate patch if that's preferred. -T