On 19 May 2014 15:13, David Herrmann <dh.herrmann@xxxxxxxxx> wrote: > Hi > > On Mon, May 19, 2014 at 3:37 PM, Thomas Wood <thomas.wood@xxxxxxxxx> wrote: >> Signed-off-by: Thomas Wood <thomas.wood@xxxxxxxxx> > > 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@xxxxxxxxxxxxxxxxxxxxx >> http://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
