[Intel-gfx] [Resend][PATCH 2/4] drm/i915: Construct all possible sdvo outputs for one sdvo port

Eric Anholt eric at anholt.net
Tue Jul 7 20:18:55 CEST 2009


On Fri, 2009-07-03 at 23:28 +0800, ling.ma at intel.com wrote:
> Some SDVO encoders will advertise multiple functions,
> We will be aware of the real one until sdvo output detection
> completes, sometime we have to change connector and encoder
> type, it looks strange for users. So we construct all possible
> sdvo outputs at initialization time.
> 
> Signed-off-by: Ma Ling <ling.ma at intel.com>
> Reviewed-by: Ian Romanick <idr at freedesktop.org>

So, I'm *really* confused about this patch.  It looks like we end up
with multiple instances of the SDVO code trying to control the SDVO[BC]
register?  Isn't the TV connector's activity going to smash the DVI
connector's activity on my SDVO device, leaving me with neither one
working?

I thought the plan for SDVO was to, while the code was only capable of
handling one active connector per encoder, just advertise one
nondescript ("Unknown") connector name on multi-connector encoders.

> ---
> According to Ian's suggestion clean up code, thanks!
> 
>  drivers/gpu/drm/i915/intel_sdvo.c |  100 ++++++++++++++++++++++++-------------
>  1 files changed, 65 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 4f0c309..53a7204 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1453,19 +1453,20 @@ intel_sdvo_hdmi_sink_detect(struct drm_connector *connector)
>  
>  static enum drm_connector_status intel_sdvo_detect(struct drm_connector *connector)
>  {
> -	u8 response[2];
> +	u16 response;
>  	u8 status;
>  	struct intel_output *intel_output = to_intel_output(connector);
> +	struct intel_sdvo_priv *sdvo_priv = intel_output->dev_priv;
>  
>  	intel_sdvo_write_cmd(intel_output, SDVO_CMD_GET_ATTACHED_DISPLAYS, NULL, 0);
>  	status = intel_sdvo_read_response(intel_output, &response, 2);
>  
> -	DRM_DEBUG("SDVO response %d %d\n", response[0], response[1]);
> +	DRM_DEBUG("SDVO response %d %d\n", response & 0xff, response >> 8);
>  
>  	if (status != SDVO_CMD_STATUS_SUCCESS)
>  		return connector_status_unknown;
>  
> -	if ((response[0] != 0) || (response[1] != 0)) {
> +	if (response & sdvo_priv->controlled_output) {
>  		intel_sdvo_hdmi_sink_detect(connector);
>  		return connector_status_connected;
>  	} else
> @@ -1866,7 +1867,9 @@ intel_sdvo_get_slave_addr(struct drm_device *dev, int output_device)
>  		return 0x72;
>  }
>  
> -bool intel_sdvo_init(struct drm_device *dev, int output_device)
> +#define MAX_SDVO_OUTPUTS_BITS 0xFFFF
> +static int intel_sdvo_outputs_init(struct drm_device *dev, int output_device,
> +				   int sdvo_output_id, int sdvo_output_flags)
>  {
>  	struct drm_connector *connector;
>  	struct intel_output *intel_output;
> @@ -1876,10 +1879,11 @@ bool intel_sdvo_init(struct drm_device *dev, int output_device)
>  	u8 ch[0x40];
>  	int i;
>  	int encoder_type;
> +	uint16_t maked_output_flags;
>  
>  	intel_output = kcalloc(sizeof(struct intel_output)+sizeof(struct intel_sdvo_priv), 1, GFP_KERNEL);
>  	if (!intel_output) {
> -		return false;
> +		return -1;
>  	}
>  
>  	sdvo_priv = (struct intel_sdvo_priv *)(intel_output + 1);
> @@ -1889,10 +1893,15 @@ bool intel_sdvo_init(struct drm_device *dev, int output_device)
>  	intel_output->type = INTEL_OUTPUT_SDVO;
>  
>  	/* setup the DDC bus. */
> -	if (output_device == SDVOB)
> -		intel_output->i2c_bus = intel_i2c_create(dev, GPIOE, "SDVOCTRL_E for SDVOB");
> -	else
> -		intel_output->i2c_bus = intel_i2c_create(dev, GPIOE, "SDVOCTRL_E for SDVOC");
> +	if (output_device == SDVOB) {
> +		sprintf(ch, "SDVOCTRL_E for SDVOB_%d", sdvo_output_id);
> +		intel_output->i2c_bus = intel_i2c_create(dev,
> +					GPIOE, (const char *)ch);
> +	} else {
> +		sprintf(ch, "SDVOCTRL_E for SDVOC_%d", sdvo_output_id);
> +		intel_output->i2c_bus = intel_i2c_create(dev,
> +					GPIOE, (const char *)ch);
> +	}
>  
>  	if (!intel_output->i2c_bus)
>  		goto err_inteloutput;
> @@ -1913,10 +1922,15 @@ bool intel_sdvo_init(struct drm_device *dev, int output_device)
>  	}
>  
>  	/* setup the DDC bus. */
> -	if (output_device == SDVOB)
> -		intel_output->ddc_bus = intel_i2c_create(dev, GPIOE, "SDVOB DDC BUS");
> -	else
> -		intel_output->ddc_bus = intel_i2c_create(dev, GPIOE, "SDVOC DDC BUS");
> +	if (output_device == SDVOB) {
> +		sprintf(ch, "SDVOB DDC BUS_%d", sdvo_output_id);
> +		intel_output->ddc_bus = intel_i2c_create(dev,
> +					GPIOE, (const char *)ch);
> +	} else {
> +		sprintf(ch, "SDVOC DDC BUS_%d", sdvo_output_id);
> +		intel_output->ddc_bus = intel_i2c_create(dev,
> +					GPIOE, (const char *)ch);
> +	}
>  
>  	if (intel_output->ddc_bus == NULL)
>  		goto err_i2c;
> @@ -1928,8 +1942,11 @@ bool intel_sdvo_init(struct drm_device *dev, int output_device)
>  	sdvo_priv->is_lvds = false;
>  	intel_sdvo_get_capabilities(intel_output, &sdvo_priv->caps);
>  
> -	if (sdvo_priv->caps.output_flags &
> -	    (SDVO_OUTPUT_TMDS0 | SDVO_OUTPUT_TMDS1)) {
> +	if (sdvo_output_flags == MAX_SDVO_OUTPUTS_BITS)
> +		sdvo_output_flags = sdvo_priv->caps.output_flags;
> +
> +	maked_output_flags = sdvo_output_flags & sdvo_priv->caps.output_flags;
> +	if (maked_output_flags & (SDVO_OUTPUT_TMDS0 | SDVO_OUTPUT_TMDS1)) {
>  		if (sdvo_priv->caps.output_flags & SDVO_OUTPUT_TMDS0)
>  			sdvo_priv->controlled_output = SDVO_OUTPUT_TMDS0;
>  		else
> @@ -1948,43 +1965,36 @@ bool intel_sdvo_init(struct drm_device *dev, int output_device)
>  						   SDVO_COLORIMETRY_RGB256);
>  			connector_type = DRM_MODE_CONNECTOR_HDMIA;
>  		}
> -	}
> -	else if (sdvo_priv->caps.output_flags & SDVO_OUTPUT_SVID0)
> -	{
> +	} else if (maked_output_flags & SDVO_OUTPUT_SVID0) {
> +
>  		sdvo_priv->controlled_output = SDVO_OUTPUT_SVID0;
>  		encoder_type = DRM_MODE_ENCODER_TVDAC;
>  		connector_type = DRM_MODE_CONNECTOR_SVIDEO;
>  		sdvo_priv->is_tv = true;
>  		intel_output->needs_tv_clock = true;
> -	}
> -	else if (sdvo_priv->caps.output_flags & SDVO_OUTPUT_RGB0)
> -	{
> +	} else if (maked_output_flags & SDVO_OUTPUT_RGB0) {
> +
>  		sdvo_priv->controlled_output = SDVO_OUTPUT_RGB0;
>  		encoder_type = DRM_MODE_ENCODER_DAC;
>  		connector_type = DRM_MODE_CONNECTOR_VGA;
> -	}
> -	else if (sdvo_priv->caps.output_flags & SDVO_OUTPUT_RGB1)
> -	{
> +	} else if (maked_output_flags & SDVO_OUTPUT_RGB1) {
> +
>  		sdvo_priv->controlled_output = SDVO_OUTPUT_RGB1;
>  		encoder_type = DRM_MODE_ENCODER_DAC;
>  		connector_type = DRM_MODE_CONNECTOR_VGA;
> -	}
> -	else if (sdvo_priv->caps.output_flags & SDVO_OUTPUT_LVDS0)
> -	{
> +	} else if (maked_output_flags & SDVO_OUTPUT_LVDS0) {
> +
>  		sdvo_priv->controlled_output = SDVO_OUTPUT_LVDS0;
>  		encoder_type = DRM_MODE_ENCODER_LVDS;
>  		connector_type = DRM_MODE_CONNECTOR_LVDS;
>  		sdvo_priv->is_lvds = true;
> -	}
> -	else if (sdvo_priv->caps.output_flags & SDVO_OUTPUT_LVDS1)
> -	{
> +	} else if (maked_output_flags & SDVO_OUTPUT_LVDS1) {
> +
>  		sdvo_priv->controlled_output = SDVO_OUTPUT_LVDS1;
>  		encoder_type = DRM_MODE_ENCODER_LVDS;
>  		connector_type = DRM_MODE_CONNECTOR_LVDS;
>  		sdvo_priv->is_lvds = true;
> -	}
> -	else
> -	{
> +	} else {
>  		unsigned char bytes[2];
>  
>  		sdvo_priv->controlled_output = 0;
> @@ -2039,7 +2049,7 @@ bool intel_sdvo_init(struct drm_device *dev, int output_device)
>  			sdvo_priv->caps.output_flags &
>  			(SDVO_OUTPUT_TMDS1 | SDVO_OUTPUT_RGB1) ? 'Y' : 'N');
>  
> -	return true;
> +	return sdvo_output_flags & (~sdvo_priv->controlled_output);
>  
>  err_i2c:
>  	if (intel_output->ddc_bus != NULL)
> @@ -2049,5 +2059,25 @@ err_i2c:
>  err_inteloutput:
>  	kfree(intel_output);
>  
> -	return false;
> +	return -1;
>  }
> +
> +bool intel_sdvo_init(struct drm_device *dev, int output_device)
> +{
> +	int sdvo_output_id;
> +	int sdvo_output_flags = MAX_SDVO_OUTPUTS_BITS;
> +	bool ret = false;
> +	sdvo_output_id = 1;
> +
> +	do {
> +		sdvo_output_flags =
> +			intel_sdvo_outputs_init(dev, output_device,
> +						sdvo_output_id++,
> +						sdvo_output_flags);
> +		if (sdvo_output_flags >= 0)
> +			ret = true;
> +	} while (sdvo_output_flags > 0);
> +
> +	return ret;
> +}
> +
-- 
Eric Anholt
eric at anholt.net                         eric.anholt at intel.com


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20090707/16e10af8/attachment.sig>


More information about the Intel-gfx mailing list