[PATCH] drm: change connector disconnected debug message to an error

[PATCH] drm: change connector disconnected debug message to an error

[Shuah Khan]

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

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

Re: [PATCH] drm: change connector disconnected debug message to an error

[Jani Nikula]

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.


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

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH] drm: change connector disconnected debug message to an error

[Shuah Khan]

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

Re: [PATCH] drm: change connector disconnected debug message to an error

[Jani Nikula]

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

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH] drm: change connector disconnected debug message to an error

[Daniel Vetter]

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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] drm: change connector disconnected debug message to an error

[Shuah Khan]

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

Re: [PATCH] drm: change connector disconnected debug message to an error

[Daniel Vetter]

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch