Re: [PATCH] drm/i915: Implement Displayport automated testing

[Todd Previte <tprevite@gmail.com>]

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 <tprevite@gmail.com>
>>>> ---
>>>>   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