Re: [PATCH 14/17] drm/i915: Add debugfs function to check connector status for compliance testing

Todd Previte
2015-02-18T23:09:09Z

After reviewing the code again, with the changes from the previous
comments, it looks like the dp_connector_is_valid() function is
superfluous. So this patch and patch 15 can be disregarded. I've added
an additional line to the config_ctl_write function to also check to
make sure the connector is active before performing the write
operations. The commit message for the monster patch where these are
implemented has been updated to reflect this change as well.

-T

On 2/18/15 10:08 AM, Todd Previte wrote:
> On 12/17/14 11:03 AM, Paulo Zanoni wrote:
>> 2014-12-10 21:53 GMT-02:00 Todd Previte<tprevite@gmail.com>:
>>> Adds a function to check the status of a Displayport connector. This
>>> function
>>> simplifies the Displayport compliance testing interface in debugfs
>>> by providing
>>> a single method for determining the status of a connector during
>>> Displayport
>>> compliance testing. This function checks whether or not the
>>> connector is a
>>> Displayport connector and whether or not that connector is active.
>>>
>> I see patches 14 and 15 rewrite some code that was added by patches 7
>> and 10. Instead of doing this, please make sure patch 7 already
>> contains your dp_connector_is_valid() function, then patch 10 just
>> calls it.
> More code for the monster patch... ;) These have been added in for V3.
>>
>> Also, please remove those "goto" and just use plain "return".
> Done. It was an attempt to make the code cleaner and have a single
> point of exit, but in reality, it's not necessary and doesn't achieve
> its intended goal. They've been replaced with returns for V3.
>
>>> Signed-off-by: Todd Previte<tprevite@gmail.com>
>>> ---
>>> drivers/gpu/drm/i915/i915_debugfs.c | 29
>>> +++++++++++++++++++++++++++++
>>> 1 file changed, 29 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>>> b/drivers/gpu/drm/i915/i915_debugfs.c
>>> index 5eb6c24..978ddd1 100644
>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>> @@ -3920,6 +3920,35 @@ static int displayport_parse_config(char
>>> *input_buffer,
>>> return status;
>>> }
>>>
>>> +static bool dp_connector_is_valid(struct drm_connector *connector,
>>> + bool check_active)
>>> +{
>>> + struct intel_encoder *intel_encoder;
>>> + struct intel_connector *intel_connector;
>>> +
>>> + intel_connector = to_intel_connector(connector);
>>> + intel_encoder = intel_connector->encoder;
>>> +
>>> + if (connector->connector_type !=
>>> DRM_MODE_CONNECTOR_DisplayPort)
>>> + goto no;
>>> +
>>> + if (intel_encoder->type != INTEL_OUTPUT_DISPLAYPORT &&
>>> + intel_encoder->type != INTEL_OUTPUT_UNKNOWN)
>>> + goto no;
>>> +
>>> + if (check_active) {
>>> + if (connector->status == connector_status_connected)
>>> + goto yes;
>>> + else
>>> + goto no;
>>> + }
>>> +
>>> +yes:
>>> + return true;
>>> +no:
>>> + return false;
>>> +}
>>> +
>>> static int displayport_config_ctl_show(struct seq_file *m, void
>>> *data)
>>> {
>>> struct drm_device *dev = m->private;
>>> --
>>> 1.9.1
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>