From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:34998 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751026AbcFXVwh (ORCPT ); Fri, 24 Jun 2016 17:52:37 -0400 Message-ID: <1466805154.4361.2.camel@redhat.com> Subject: Re: [PATCH 1/2] drm/radeon: Poll for both connect/disconnect on analog connectors From: Lyude To: xorg-driver-ati@lists.freedesktop.org, amd-gfx@lists.freedesktop.org Cc: stable@vger.kernel.org, Alex Deucher , Christian =?ISO-8859-1?Q?K=F6nig?= , David Airlie , "open list:RADEON and AMDGPU DRM DRIVERS" , open list Date: Fri, 24 Jun 2016 17:52:34 -0400 In-Reply-To: <1466804762-14526-1-git-send-email-cpaul@redhat.com> References: <1466804762-14526-1-git-send-email-cpaul@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: stable-owner@vger.kernel.org List-ID: Whoops, very sorry about this! I ran git send-email, and it looks like I had forgotten to remove some other patches in my patches/ folder. Going to resend this to avoid confusing anyone trying to review this. On Fri, 2016-06-24 at 17:45 -0400, Lyude wrote: > DRM_CONNECTOR_POLL_CONNECT only enables polling for connections, not > disconnections. Because of this, we end up losing hotplug polling for > analog connectors once they get connected. > > Easy way to reproduce: >  - Grab a machine with a radeon GPU and a VGA port >  - Plug a monitor into the VGA port, wait for it to update the > connector >    from disconnected to connected >  - Disconnect the monitor on VGA, a hotplug event is never sent for > the >    removal of the connector. > > Originally, only using DRM_CONNECTOR_POLL_CONNECT might have been a > good > idea since doing VGA polling can sometimes result in having to mess > with > the DAC voltages to figure out whether or not there's actually > something > there since VGA doesn't have HPD. Doing this would have the potential > of > showing visible artifacts on the screen every time we ran a poll > while a > VGA display was connected. Luckily, radeon_vga_detect() only resorts > to > this sort of polling if the poll is forced, and DRM's polling helper > doesn't force it's polls. > > Additionally, this removes some assignments to connector->polled that > weren't actually doing anything. > > Cc: stable@vger.kernel.org > Signed-off-by: Lyude > --- >  drivers/gpu/drm/radeon/radeon_connectors.c | 15 +++++++++------ >  1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c > b/drivers/gpu/drm/radeon/radeon_connectors.c > index 81a63d7..b79f3b0 100644 > --- a/drivers/gpu/drm/radeon/radeon_connectors.c > +++ b/drivers/gpu/drm/radeon/radeon_connectors.c > @@ -2064,7 +2064,6 @@ radeon_add_atom_connector(struct drm_device > *dev, >      RADEON_OU > TPUT_CSC_BYPASS); >   /* no HPD on analog connectors */ >   radeon_connector->hpd.hpd = RADEON_HPD_NONE; > - connector->polled = > DRM_CONNECTOR_POLL_CONNECT; >   connector->interlace_allowed = true; >   connector->doublescan_allowed = true; >   break; > @@ -2314,8 +2313,10 @@ radeon_add_atom_connector(struct drm_device > *dev, >   } >   >   if (radeon_connector->hpd.hpd == RADEON_HPD_NONE) { > - if (i2c_bus->valid) > - connector->polled = > DRM_CONNECTOR_POLL_CONNECT; > + if (i2c_bus->valid) { > + connector->polled = > DRM_CONNECTOR_POLL_CONNECT | > +                     DRM_CONNECTOR_POLL_DISCO > NNECT; > + } >   } else >   connector->polled = DRM_CONNECTOR_POLL_HPD; >   > @@ -2391,7 +2392,6 @@ radeon_add_legacy_connector(struct drm_device > *dev, >         1); >   /* no HPD on analog connectors */ >   radeon_connector->hpd.hpd = RADEON_HPD_NONE; > - connector->polled = DRM_CONNECTOR_POLL_CONNECT; >   connector->interlace_allowed = true; >   connector->doublescan_allowed = true; >   break; > @@ -2476,10 +2476,13 @@ radeon_add_legacy_connector(struct drm_device > *dev, >   } >   >   if (radeon_connector->hpd.hpd == RADEON_HPD_NONE) { > - if (i2c_bus->valid) > - connector->polled = > DRM_CONNECTOR_POLL_CONNECT; > + if (i2c_bus->valid) { > + connector->polled = > DRM_CONNECTOR_POLL_CONNECT | > +                     DRM_CONNECTOR_POLL_DISCO > NNECT; > + } >   } else >   connector->polled = DRM_CONNECTOR_POLL_HPD; > + >   connector->display_info.subpixel_order = subpixel_order; >   drm_connector_register(connector); >  } -- Cheers, Lyude