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

From: Jani Nikula <jani.nikula@linux.intel.com>
To: daniel.vetter@intel.com, seanpaul@chromium.org, airlied@linux.ie
Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	Shuah Khan <shuahkh@osg.samsung.com>
Subject: Re: [PATCH] drm: change connector disconnected debug message to an error
Date: Fri, 03 Feb 2017 09:59:35 +0200	[thread overview]
Message-ID: <87vasryaag.fsf@intel.com> (raw)
In-Reply-To: <9d3783d1-b8f8-bd82-3141-1897980197da@osg.samsung.com>

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel