| Message ID | 1418255597-4716-15-git-send-email-tprevite@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. Also, please remove those "goto" and just use plain "return". > 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
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 >
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 >> >
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;
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. Signed-off-by: Todd Previte <tprevite@gmail.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)