From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lyude Subject: Re: [PATCH 1/2] drm/radeon: Poll for both connect/disconnect on analog connectors Date: Fri, 24 Jun 2016 17:52:34 -0400 Message-ID: <1466805154.4361.2.camel@redhat.com> References: <1466804762-14526-1-git-send-email-cpaul@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <1466804762-14526-1-git-send-email-cpaul@redhat.com> Sender: stable-owner@vger.kernel.org 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 List-Id: dri-devel@lists.freedesktop.org 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. >=20 > Easy way to reproduce: > =C2=A0- Grab a machine with a radeon GPU and a VGA port > =C2=A0- Plug a monitor into the VGA port, wait for it to update the > connector > =C2=A0=C2=A0=C2=A0from disconnected to connected > =C2=A0- Disconnect the monitor on VGA, a hotplug event is never sent = for > the > =C2=A0=C2=A0=C2=A0removal of the connector. >=20 > 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. >=20 > Additionally, this removes some assignments to connector->polled that > weren't actually doing anything. >=20 > Cc: stable@vger.kernel.org > Signed-off-by: Lyude > --- > =C2=A0drivers/gpu/drm/radeon/radeon_connectors.c | 15 +++++++++------ > =C2=A01 file changed, 9 insertions(+), 6 deletions(-) >=20 > 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, > =C2=A0 =C2=A0=C2=A0=C2=A0RADEON_OU > TPUT_CSC_BYPASS); > =C2=A0 /* no HPD on analog connectors */ > =C2=A0 radeon_connector->hpd.hpd =3D RADEON_HPD_NONE; > - connector->polled =3D > DRM_CONNECTOR_POLL_CONNECT; > =C2=A0 connector->interlace_allowed =3D true; > =C2=A0 connector->doublescan_allowed =3D true; > =C2=A0 break; > @@ -2314,8 +2313,10 @@ radeon_add_atom_connector(struct drm_device > *dev, > =C2=A0 } > =C2=A0 > =C2=A0 if (radeon_connector->hpd.hpd =3D=3D RADEON_HPD_NONE) { > - if (i2c_bus->valid) > - connector->polled =3D > DRM_CONNECTOR_POLL_CONNECT; > + if (i2c_bus->valid) { > + connector->polled =3D > DRM_CONNECTOR_POLL_CONNECT | > + =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0DRM_CONNECTOR_POL= L_DISCO > NNECT; > + } > =C2=A0 } else > =C2=A0 connector->polled =3D DRM_CONNECTOR_POLL_HPD; > =C2=A0 > @@ -2391,7 +2392,6 @@ radeon_add_legacy_connector(struct drm_device > *dev, > =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A01); > =C2=A0 /* no HPD on analog connectors */ > =C2=A0 radeon_connector->hpd.hpd =3D RADEON_HPD_NONE; > - connector->polled =3D DRM_CONNECTOR_POLL_CONNECT; > =C2=A0 connector->interlace_allowed =3D true; > =C2=A0 connector->doublescan_allowed =3D true; > =C2=A0 break; > @@ -2476,10 +2476,13 @@ radeon_add_legacy_connector(struct drm_device > *dev, > =C2=A0 } > =C2=A0 > =C2=A0 if (radeon_connector->hpd.hpd =3D=3D RADEON_HPD_NONE) { > - if (i2c_bus->valid) > - connector->polled =3D > DRM_CONNECTOR_POLL_CONNECT; > + if (i2c_bus->valid) { > + connector->polled =3D > DRM_CONNECTOR_POLL_CONNECT | > + =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0DRM_CONNECTOR_POL= L_DISCO > NNECT; > + } > =C2=A0 } else > =C2=A0 connector->polled =3D DRM_CONNECTOR_POLL_HPD; > + > =C2=A0 connector->display_info.subpixel_order =3D subpixel_order; > =C2=A0 drm_connector_register(connector); > =C2=A0} --=20 Cheers, Lyude