diff mbox

drm/radeon: hotplug of passive dp to dvi|hdmi|vga adaptor

Message ID CADnq5_OC2hHprRdA1hQTAuwQkeW+Fd=tZe=R1Z-m26TpvZv3dQ@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Alex Deucher July 19, 2012, 9:34 p.m. UTC
On Thu, Jul 19, 2012 at 5:12 PM,  <j.glisse@gmail.com> wrote:
> From: Jerome Glisse <jglisse@redhat.com>
>
> We should not turn off the connector neither try to retrain DP link
> if a passive DP adaptor is connected to a DP port.
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Jerome Glisse <jglisse@redhat.com>

Looks good although we should probably wait to assign
radeon_connector_atom_dig until we know we have a digital connector.
How about the attached patches (also a follow up on your patch from
yesterday)?

Alex

> ---
>  drivers/gpu/drm/radeon/radeon_connectors.c |   22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
> index 2914c57..890cf1d 100644
> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> @@ -49,6 +49,7 @@ void radeon_connector_hotplug(struct drm_connector *connector)
>         struct drm_device *dev = connector->dev;
>         struct radeon_device *rdev = dev->dev_private;
>         struct radeon_connector *radeon_connector = to_radeon_connector(connector);
> +       struct radeon_connector_atom_dig *radeon_dig_connector = radeon_connector->con_priv;
>
>         /* bail if the connector does not have hpd pin, e.g.,
>          * VGA, TV, etc.
> @@ -62,15 +63,32 @@ void radeon_connector_hotplug(struct drm_connector *connector)
>         if (connector->dpms != DRM_MODE_DPMS_ON)
>                 return;
>
> +       /* don't do anything is sink is not display port
> +        * (passive dp->(dvi|hdmi|vga) adaptor
> +        */
> +       if (radeon_dig_connector->dp_sink_type != CONNECTOR_OBJECT_ID_DISPLAYPORT) {
> +               return;
> +       }
> +
>         /* just deal with DP (not eDP) here. */
>         if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
>                 int saved_dpms = connector->dpms;
>
>                 /* Only turn off the display it it's physically disconnected */
> -               if (!radeon_hpd_sense(rdev, radeon_connector->hpd.hpd))
> +               if (!radeon_hpd_sense(rdev, radeon_connector->hpd.hpd)) {
>                         drm_helper_connector_dpms(connector, DRM_MODE_DPMS_OFF);
> -               else if (radeon_dp_needs_link_train(radeon_connector))
> +               } else if (radeon_dp_needs_link_train(radeon_connector)) {
> +
> +                       /* first get sink type as it's reset after unplug */
> +                       radeon_dig_connector->dp_sink_type = radeon_dp_getsinktype(radeon_connector);
> +                       /* don't do anything is sink is not display port
> +                        * (passive dp->(dvi|hdmi|vga) adaptor
> +                        */
> +                       if (radeon_dig_connector->dp_sink_type != CONNECTOR_OBJECT_ID_DISPLAYPORT) {
> +                               return;
> +                       }
>                         drm_helper_connector_dpms(connector, DRM_MODE_DPMS_ON);
> +               }
>                 connector->dpms = saved_dpms;
>         }
>  }
> --
> 1.7.10.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Comments

Jerome Glisse July 20, 2012, 2:25 a.m. UTC | #1
On Thu, Jul 19, 2012 at 5:34 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Thu, Jul 19, 2012 at 5:12 PM,  <j.glisse@gmail.com> wrote:
>> From: Jerome Glisse <jglisse@redhat.com>
>>
>> We should not turn off the connector neither try to retrain DP link
>> if a passive DP adaptor is connected to a DP port.
>>
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Jerome Glisse <jglisse@redhat.com>
>
> Looks good although we should probably wait to assign
> radeon_connector_atom_dig until we know we have a digital connector.
> How about the attached patches (also a follow up on your patch from
> yesterday)?
>
> Alex

Yes looks better.

Cheers,
Jerome

>> ---
>>  drivers/gpu/drm/radeon/radeon_connectors.c |   22 ++++++++++++++++++++--
>>  1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
>> index 2914c57..890cf1d 100644
>> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
>> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
>> @@ -49,6 +49,7 @@ void radeon_connector_hotplug(struct drm_connector *connector)
>>         struct drm_device *dev = connector->dev;
>>         struct radeon_device *rdev = dev->dev_private;
>>         struct radeon_connector *radeon_connector = to_radeon_connector(connector);
>> +       struct radeon_connector_atom_dig *radeon_dig_connector = radeon_connector->con_priv;
>>
>>         /* bail if the connector does not have hpd pin, e.g.,
>>          * VGA, TV, etc.
>> @@ -62,15 +63,32 @@ void radeon_connector_hotplug(struct drm_connector *connector)
>>         if (connector->dpms != DRM_MODE_DPMS_ON)
>>                 return;
>>
>> +       /* don't do anything is sink is not display port
>> +        * (passive dp->(dvi|hdmi|vga) adaptor
>> +        */
>> +       if (radeon_dig_connector->dp_sink_type != CONNECTOR_OBJECT_ID_DISPLAYPORT) {
>> +               return;
>> +       }
>> +
>>         /* just deal with DP (not eDP) here. */
>>         if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
>>                 int saved_dpms = connector->dpms;
>>
>>                 /* Only turn off the display it it's physically disconnected */
>> -               if (!radeon_hpd_sense(rdev, radeon_connector->hpd.hpd))
>> +               if (!radeon_hpd_sense(rdev, radeon_connector->hpd.hpd)) {
>>                         drm_helper_connector_dpms(connector, DRM_MODE_DPMS_OFF);
>> -               else if (radeon_dp_needs_link_train(radeon_connector))
>> +               } else if (radeon_dp_needs_link_train(radeon_connector)) {
>> +
>> +                       /* first get sink type as it's reset after unplug */
>> +                       radeon_dig_connector->dp_sink_type = radeon_dp_getsinktype(radeon_connector);
>> +                       /* don't do anything is sink is not display port
>> +                        * (passive dp->(dvi|hdmi|vga) adaptor
>> +                        */
>> +                       if (radeon_dig_connector->dp_sink_type != CONNECTOR_OBJECT_ID_DISPLAYPORT) {
>> +                               return;
>> +                       }
>>                         drm_helper_connector_dpms(connector, DRM_MODE_DPMS_ON);
>> +               }
>>                 connector->dpms = saved_dpms;
>>         }
>>  }
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

From 9de839f3907cf04febdeed0e92528c729f4e735c Mon Sep 17 00:00:00 2001
From: Jerome Glisse <jglisse@redhat.com>
Date: Thu, 19 Jul 2012 17:25:55 -0400
Subject: [PATCH 2/2] drm/radeon: on hotplug force link training to happen
 (v2)

To have DP behave like VGA/DVI we need to retrain the link
on hotplug. For this to happen we need to force link
training to happen by setting connector dpms to off
before asking it turning it on again.

v2: agd5f
- drop the dp_get_link_status() change in atombios_dp.c
  for now.  We still need the dpms OFF change.

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/radeon_connectors.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index 3524f17..895e628 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -79,10 +79,16 @@  void radeon_connector_hotplug(struct drm_connector *connector)
 		if (dig_connector->dp_sink_type == CONNECTOR_OBJECT_ID_DISPLAYPORT) {
 			int saved_dpms = connector->dpms;
 			/* Only turn off the display if it's physically disconnected */
-			if (!radeon_hpd_sense(rdev, radeon_connector->hpd.hpd))
+			if (!radeon_hpd_sense(rdev, radeon_connector->hpd.hpd)) {
 				drm_helper_connector_dpms(connector, DRM_MODE_DPMS_OFF);
-			else if (radeon_dp_needs_link_train(radeon_connector))
+			} else if (radeon_dp_needs_link_train(radeon_connector)) {
+				/* set it to OFF so that drm_helper_connector_dpms()
+				 * won't return immediately since the current state
+				 * is ON at this point.
+				 */
+				connector->dpms = DRM_MODE_DPMS_OFF;
 				drm_helper_connector_dpms(connector, DRM_MODE_DPMS_ON);
+			}
 			connector->dpms = saved_dpms;
 		}
 	}
-- 
1.7.7.5