linux-kernel - Re: [PATCH 1/2] drm/radeon: Poll for both connect/disconnect on analog connectors

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1466805154.4361.2.camel@redhat.com>
Date:	Fri, 24 Jun 2016 17:52:34 -0400
From:	Lyude <cpaul@...hat.com>
To:	xorg-driver-ati@...ts.freedesktop.org,
	amd-gfx@...ts.freedesktop.org
Cc:	stable@...r.kernel.org, Alex Deucher <alexander.deucher@....com>,
	Christian König <christian.koenig@....com>,
	David Airlie <airlied@...ux.ie>,
	"open list:RADEON and AMDGPU DRM DRIVERS" 
	<dri-devel@...ts.freedesktop.org>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] drm/radeon: Poll for both connect/disconnect on
 analog connectors

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@...r.kernel.org
> Signed-off-by: Lyude <cpaul@...hat.com>
> ---
>  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