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

[Todd Previte <tprevite@gmail.com>]

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