[Intel-gfx] [PATCH] drm/sysfs: expose the "force" connector attribute

[Thomas Wood]

On 19 May 2014 15:13, David Herrmann <dh.herrmann at gmail.com> wrote:
> Hi
>
> On Mon, May 19, 2014 at 3:37 PM, Thomas Wood <thomas.wood at intel.com> wrote:
>> Signed-off-by: Thomas Wood <thomas.wood at intel.com>
>
> The commit-msg lacks any discussion why this change is done. What is
> the reason to do that? Isn't the kernel-command-line enough? Why is
> this a regular feature instead of a debugfs attribute?


It was intended as a debug/testing feature to allow tests in
intel-gpu-tools to enable or disable connectors:

http://lists.freedesktop.org/archives/intel-gfx/2014-May/045556.html


I'll update the commit message for the next version of the patch.



>
>> ---
>>  drivers/gpu/drm/drm_sysfs.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 49 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
>> index c22c309..257816e 100644
>> --- a/drivers/gpu/drm/drm_sysfs.c
>> +++ b/drivers/gpu/drm/drm_sysfs.c
>> @@ -338,11 +338,60 @@ static ssize_t select_subconnector_show(struct device *device,
>>                         drm_get_dvi_i_select_name((int)subconnector));
>>  }
>>
>> +static ssize_t force_show(struct device *device,
>> +                         struct device_attribute *attr,
>> +                         char *buf)
>> +{
>> +       struct drm_connector *connector = to_drm_connector(device);
>> +       char *status;
>
> "const char *" or gcc might warn on string assignments.
>
>> +
>> +       switch (connector->force) {
>> +       case DRM_FORCE_ON:
>> +               status = "on";
>> +               break;
>> +
>> +       case DRM_FORCE_ON_DIGITAL:
>> +               status = "digital";
>> +               break;
>> +
>> +       case DRM_FORCE_OFF:
>> +               status = "off";
>> +               break;
>> +
>> +       case DRM_FORCE_UNSPECIFIED:
>> +               status = "unspecified";
>> +               break;
>> +
>> +       default:
>> +               return 0;
>> +       }
>> +
>> +       return snprintf(buf, PAGE_SIZE, "%s\n", status);
>> +}
>> +
>> +ssize_t force_store(struct device *device, struct device_attribute *attr,
>> +                   const char *buf, size_t count)
>> +{
>> +       struct drm_connector *connector = to_drm_connector(device);
>> +
>> +       if (strcmp(buf, "on") == 0)
>> +               connector->force = DRM_FORCE_ON;
>> +       else if (strcmp(buf, "digital") == 0)
>> +               connector->force = DRM_FORCE_ON_DIGITAL;
>> +       else if (strcmp(buf, "off") == 0)
>> +               connector->force = DRM_FORCE_OFF;
>> +       else if (strcmp(buf, "unspecified") == 0)
>> +               connector->force = DRM_FORCE_UNSPECIFIED;
>
> else
>         return -EINVAL;
>
>
> This really looks like a debug-feature to me. If it's a real feature,
> we _must_ rescan connectors here, otherwise it seems odd writing "on"
> into that file but nothing happens until the next modeset.
>
> Thanks
> David
>
>> +
>> +       return count;
>> +}
>> +
>>  static struct device_attribute connector_attrs[] = {
>>         __ATTR_RO(status),
>>         __ATTR_RO(enabled),
>>         __ATTR_RO(dpms),
>>         __ATTR_RO(modes),
>> +       __ATTR_RW(force),
>>  };
>>
>>  /* These attributes are for both DVI-I connectors and all types of tv-out. */
>> --
>> 1.9.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel