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

[David Herrmann]

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?

> ---
>  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