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

[yakui_zhao]

On Sun, 2009-05-31 at 17:00 +0800, Ma Ling 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
Very good idea to construct all the possible sdvo outputs at
initialization time.

But I have two questions about this patch:

a. It seems that the i2c bus will be created several times if it 
is a multi-function SDVO device. Of course the slave device is also
created several times.

How about creating the output according to the capability of SDVO
device? That means that we detect the capability of SDVO device and then
create the output.

b. the synchronization between the different should be considered. Only
one output can be activated for the multi-function SDVO outputs. Is this
considered?

Best regards.

> 
> Signed-off-by: Ma Ling <ling.ma at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sdvo.c |   75 +++++++++++++++++++++++++-----------
>  1 files changed, 52 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index f3ef6bf..b434ee5 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1741,7 +1741,10 @@ static struct i2c_algorithm intel_sdvo_i2c_bit_algo = {
>  	.master_xfer	= intel_sdvo_master_xfer,
>  };
>  
> -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 intel_flags)
>  {
>  	struct drm_connector *connector;
>  	struct intel_output *intel_output;
> @@ -1755,7 +1758,7 @@ bool intel_sdvo_init(struct drm_device *dev, int output_device)
>  
>  	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);
> @@ -1770,6 +1773,9 @@ bool intel_sdvo_init(struct drm_device *dev, int output_device)
>  	if (!i2cbus)
>  		goto err_inteloutput;
>  
> +	sprintf(i2cbus->adapter.name, "%s_%d",
> +			i2cbus->adapter.name, sdvo_output_id);
> +
>  	sdvo_priv->i2c_bus = i2cbus;
>  
>  	if (output_device == SDVOB) {
> @@ -1802,6 +1808,9 @@ bool intel_sdvo_init(struct drm_device *dev, int output_device)
>  	if (ddcbus == NULL)
>  		goto err_i2c;
>  
> +	sprintf(ddcbus->adapter.name, "%s_%d",
> +			ddcbus->adapter.name, sdvo_output_id);
> +
>  	intel_sdvo_i2c_bit_algo.functionality =
>  		intel_output->i2c_bus->adapter.algo->functionality;
>  	ddcbus->adapter.algo = &intel_sdvo_i2c_bit_algo;
> @@ -1811,7 +1820,10 @@ 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 &
> +	if (intel_flags == MAX_SDVO_OUTPUTS_BITS)
> +		intel_flags = sdvo_priv->caps.output_flags;
> +
> +	if ((sdvo_priv->caps.output_flags & intel_flags) &
>  	    (SDVO_OUTPUT_TMDS0 | SDVO_OUTPUT_TMDS1)) {
>  		if (sdvo_priv->caps.output_flags & SDVO_OUTPUT_TMDS0)
>  			sdvo_priv->controlled_output = SDVO_OUTPUT_TMDS0;
> @@ -1831,43 +1843,41 @@ 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 ((sdvo_priv->caps.output_flags & intel_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 ((sdvo_priv->caps.output_flags & intel_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 ((sdvo_priv->caps.output_flags & intel_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 ((sdvo_priv->caps.output_flags & intel_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 ((sdvo_priv->caps.output_flags & intel_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;
> @@ -1921,7 +1931,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 intel_flags & (~sdvo_priv->controlled_output);
>  
>  err_i2c:
>  	if (ddcbus != NULL)
> @@ -1930,5 +1940,24 @@ 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 intel_flags = MAX_SDVO_OUTPUTS_BITS;
> +	bool ret = false;
> +
> +	for (sdvo_output_id = 0; intel_flags > 0; sdvo_output_id++) {
> +
> +		intel_flags =
> +			intel_sdvo_outputs_init(dev, output_device,
> +						sdvo_output_id, intel_flags);
> +		if (intel_flags >= 0)
> +			ret = true;
> +	}
> +
> +	return ret;
> +
>  }