| Message ID | 20170202025904.25810-1-shuahkh@osg.samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/02/2017 01:32 AM, Jani Nikula wrote: > On Thu, 02 Feb 2017, Shuah Khan <shuahkh@osg.samsung.com> wrote: >> Change drm_helper_probe_single_connector_modes() to print an error to >> report connector disconnected status instead of a debug message. >> >> When this condition occurs, application doesn't know the real error and >> reports it as driver lacking support for mode setting. Change it to an >> error to make it easier to debug. > > Please explain what makes this condition an error. Connectors get > connected and disconnected, business as usual, why should this be an > error? > > BR, > Jani. Disconnecting connector itself isn't an error. When user-space tries to access it, it would be useful to report the status that the connector is disconnected. I use embedded system(s) that don't like it when HDMI is hot added or removed. Also, because of return power, it is safer to disconnect HDMI and then apply power to the board. It chased a few libdrm and user-space dead ends before I enabled drm debug and was able to fix the real issue, which is a disconnected cable. User-space prints rather confusing messages as it doesn't really know the disconnected status as it isn't returned to it. I figured it might be a good idea to at least print a message and this can be a notice or info instead of an error. I do think its is worth while in some cases. thanks, -- Shuah > > >> >> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com> >> --- >> drivers/gpu/drm/drm_probe_helper.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c >> index ac953f0..6472b7f 100644 >> --- a/drivers/gpu/drm/drm_probe_helper.c >> +++ b/drivers/gpu/drm/drm_probe_helper.c >> @@ -282,8 +282,8 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, >> dev->mode_config.poll_running = drm_kms_helper_poll; >> >> if (connector->status == connector_status_disconnected) { >> - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n", >> - connector->base.id, connector->name); >> + DRM_ERROR("[CONNECTOR:%d:%s] disconnected\n", >> + connector->base.id, connector->name); >> drm_mode_connector_update_edid_property(connector, NULL); >> verbose_prune = false; >> goto prune; >
On Thu, 02 Feb 2017, Shuah Khan <shuahkh@osg.samsung.com> wrote: > On 02/02/2017 01:32 AM, Jani Nikula wrote: >> On Thu, 02 Feb 2017, Shuah Khan <shuahkh@osg.samsung.com> wrote: >>> Change drm_helper_probe_single_connector_modes() to print an error to >>> report connector disconnected status instead of a debug message. >>> >>> When this condition occurs, application doesn't know the real error and >>> reports it as driver lacking support for mode setting. Change it to an >>> error to make it easier to debug. >> >> Please explain what makes this condition an error. Connectors get >> connected and disconnected, business as usual, why should this be an >> error? >> >> BR, >> Jani. > > Disconnecting connector itself isn't an error. When user-space tries > to access it, it would be useful to report the status that the connector > is disconnected. > > I use embedded system(s) that don't like it when HDMI is hot added or > removed. Also, because of return power, it is safer to disconnect HDMI > and then apply power to the board. It chased a few libdrm and user-space > dead ends before I enabled drm debug and was able to fix the real issue, > which is a disconnected cable. > > User-space prints rather confusing messages as it doesn't really know > the disconnected status as it isn't returned to it. > > I figured it might be a good idea to at least print a message and this can > be a notice or info instead of an error. I do think its is worth while in > some cases. Well, I still think having a debug message for debugging is the way to go, and that's what we have. Perhaps it is the userspace messages that need improvement instead? BR, Jani. > > thanks, > -- Shuah > > >> >> >>> >>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com> >>> --- >>> drivers/gpu/drm/drm_probe_helper.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c >>> index ac953f0..6472b7f 100644 >>> --- a/drivers/gpu/drm/drm_probe_helper.c >>> +++ b/drivers/gpu/drm/drm_probe_helper.c >>> @@ -282,8 +282,8 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, >>> dev->mode_config.poll_running = drm_kms_helper_poll; >>> >>> if (connector->status == connector_status_disconnected) { >>> - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n", >>> - connector->base.id, connector->name); >>> + DRM_ERROR("[CONNECTOR:%d:%s] disconnected\n", >>> + connector->base.id, connector->name); >>> drm_mode_connector_update_edid_property(connector, NULL); >>> verbose_prune = false; >>> goto prune; >> >
On Thu, Feb 02, 2017 at 10:25:44AM -0700, Shuah Khan wrote: > On 02/02/2017 01:32 AM, Jani Nikula wrote: > > On Thu, 02 Feb 2017, Shuah Khan <shuahkh@osg.samsung.com> wrote: > >> Change drm_helper_probe_single_connector_modes() to print an error to > >> report connector disconnected status instead of a debug message. > >> > >> When this condition occurs, application doesn't know the real error and > >> reports it as driver lacking support for mode setting. Change it to an > >> error to make it easier to debug. > > > > Please explain what makes this condition an error. Connectors get > > connected and disconnected, business as usual, why should this be an > > error? > > > > BR, > > Jani. > > Disconnecting connector itself isn't an error. When user-space tries > to access it, it would be useful to report the status that the connector > is disconnected. > > I use embedded system(s) that don't like it when HDMI is hot added or > removed. Also, because of return power, it is safer to disconnect HDMI > and then apply power to the board. It chased a few libdrm and user-space > dead ends before I enabled drm debug and was able to fix the real issue, > which is a disconnected cable. > > User-space prints rather confusing messages as it doesn't really know > the disconnected status as it isn't returned to it. > > I figured it might be a good idea to at least print a message and this can > be a notice or info instead of an error. I do think its is worth while in > some cases. This sounds like a very specific use-case you have here, and it can easily be supported by a small deamon in userspace (only on debug builds ofc) that tell you that someone unplugged the screen when it shouldn't have been. Because upstream runs also on non-embedded systems, where unplugging is normal, and we definitely don't want to spam dmesg. -Daniel
On 02/03/2017 01:06 AM, Daniel Vetter wrote: > On Thu, Feb 02, 2017 at 10:25:44AM -0700, Shuah Khan wrote: >> On 02/02/2017 01:32 AM, Jani Nikula wrote: >>> On Thu, 02 Feb 2017, Shuah Khan <shuahkh@osg.samsung.com> wrote: >>>> Change drm_helper_probe_single_connector_modes() to print an error to >>>> report connector disconnected status instead of a debug message. >>>> >>>> When this condition occurs, application doesn't know the real error and >>>> reports it as driver lacking support for mode setting. Change it to an >>>> error to make it easier to debug. >>> >>> Please explain what makes this condition an error. Connectors get >>> connected and disconnected, business as usual, why should this be an >>> error? >>> >>> BR, >>> Jani. >> >> Disconnecting connector itself isn't an error. When user-space tries >> to access it, it would be useful to report the status that the connector >> is disconnected. >> >> I use embedded system(s) that don't like it when HDMI is hot added or >> removed. Also, because of return power, it is safer to disconnect HDMI >> and then apply power to the board. It chased a few libdrm and user-space >> dead ends before I enabled drm debug and was able to fix the real issue, >> which is a disconnected cable. >> >> User-space prints rather confusing messages as it doesn't really know >> the disconnected status as it isn't returned to it. >> >> I figured it might be a good idea to at least print a message and this can >> be a notice or info instead of an error. I do think its is worth while in >> some cases. > > This sounds like a very specific use-case you have here, and it can easily > be supported by a small deamon in userspace (only on debug builds ofc) > that tell you that someone unplugged the screen when it shouldn't have > been. drm_helper_probe_single_connector_modes() finds the condition and doesn't have a means to return it to the user-space. Instead of error or debug message, would it be useful to add a trace event to report status of connector to drm_helper_probe_single_connector_modes() Trace could be triggered as needed and turned off. Please let me know what you think of this idea? If it sounds useful, I can add it. > > Because upstream runs also on non-embedded systems, where unplugging is > normal, and we definitely don't want to spam dmesg. > -Daniel > thanks, -- Shuah
On Fri, Feb 10, 2017 at 09:29:07AM -0700, Shuah Khan wrote: > On 02/03/2017 01:06 AM, Daniel Vetter wrote: > > On Thu, Feb 02, 2017 at 10:25:44AM -0700, Shuah Khan wrote: > >> On 02/02/2017 01:32 AM, Jani Nikula wrote: > >>> On Thu, 02 Feb 2017, Shuah Khan <shuahkh@osg.samsung.com> wrote: > >>>> Change drm_helper_probe_single_connector_modes() to print an error to > >>>> report connector disconnected status instead of a debug message. > >>>> > >>>> When this condition occurs, application doesn't know the real error and > >>>> reports it as driver lacking support for mode setting. Change it to an > >>>> error to make it easier to debug. > >>> > >>> Please explain what makes this condition an error. Connectors get > >>> connected and disconnected, business as usual, why should this be an > >>> error? > >>> > >>> BR, > >>> Jani. > >> > >> Disconnecting connector itself isn't an error. When user-space tries > >> to access it, it would be useful to report the status that the connector > >> is disconnected. > >> > >> I use embedded system(s) that don't like it when HDMI is hot added or > >> removed. Also, because of return power, it is safer to disconnect HDMI > >> and then apply power to the board. It chased a few libdrm and user-space > >> dead ends before I enabled drm debug and was able to fix the real issue, > >> which is a disconnected cable. > >> > >> User-space prints rather confusing messages as it doesn't really know > >> the disconnected status as it isn't returned to it. > >> > >> I figured it might be a good idea to at least print a message and this can > >> be a notice or info instead of an error. I do think its is worth while in > >> some cases. > > > > This sounds like a very specific use-case you have here, and it can easily > > be supported by a small deamon in userspace (only on debug builds ofc) > > that tell you that someone unplugged the screen when it shouldn't have > > been. > > drm_helper_probe_single_connector_modes() finds the condition and doesn't > have a means to return it to the user-space. Erhm, we send out the uevent for this, and userspace can react. If that's not working, then we need to fix this bug, not add more uapi interfaces on top ... -Daniel > > Instead of error or debug message, would it be useful to add a trace event > to report status of connector to drm_helper_probe_single_connector_modes() > Trace could be triggered as needed and turned off. > > Please let me know what you think of this idea? If it sounds useful, I can > add it. > > > > > Because upstream runs also on non-embedded systems, where unplugging is > > normal, and we definitely don't want to spam dmesg. > > -Daniel > > > > thanks, > -- Shuah > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index ac953f0..6472b7f 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -282,8 +282,8 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, dev->mode_config.poll_running = drm_kms_helper_poll; if (connector->status == connector_status_disconnected) { - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n", - connector->base.id, connector->name); + DRM_ERROR("[CONNECTOR:%d:%s] disconnected\n", + connector->base.id, connector->name); drm_mode_connector_update_edid_property(connector, NULL); verbose_prune = false; goto prune;
Change drm_helper_probe_single_connector_modes() to print an error to report connector disconnected status instead of a debug message. When this condition occurs, application doesn't know the real error and reports it as driver lacking support for mode setting. Change it to an error to make it easier to debug. Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com> --- drivers/gpu/drm/drm_probe_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)