[Intel-gfx] [Resend][PATCH 3/4] drm/i915: Construct all possible sdvo outputs for one sdvo port
Ian Romanick
idr at freedesktop.org
Mon Jun 29 18:34:59 CEST 2009
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
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>
> ---
> update against with latest version
>
> drivers/gpu/drm/i915/intel_sdvo.c | 99 ++++++++++++++++++++++++-------------
> 1 files changed, 65 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 0407889..6a2520e 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1449,19 +1449,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
> @@ -1861,7 +1862,10 @@ 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;
> @@ -1874,7 +1878,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);
> @@ -1884,10 +1888,13 @@ 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, "%s_%d", "SDVOCTRL_E for SDVOB", sdvo_output_id);
> + intel_output->i2c_bus = intel_i2c_create(dev, GPIOE, (const char *)ch);
> + } else {
> + sprintf(ch, "%s_%d", "SDVOCTRL_E for SDVOC", sdvo_output_id);
> + intel_output->i2c_bus = intel_i2c_create(dev, GPIOE, (const char *)ch);
> + }
Either make these 'sprintf(ch, "SDVOCTRL_E for SDVOC %d",
sdvo_output_id);' or
static const char fmt[] = "SDVOCTRL_E for SDVO%c_%d";
...
sprintf(ch, fmt, 'C', sdvo_output_id);
>
> if (!intel_output->i2c_bus)
> goto err_inteloutput;
> @@ -1908,10 +1915,13 @@ 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, "%s_%d", "SDVOB DDC BUS", sdvo_output_id);
> + intel_output->ddc_bus = intel_i2c_create(dev, GPIOE, (const char *)ch);
> + } else {
> + sprintf(ch, "%s_%d", "SDVOC DDC BUS", sdvo_output_id);
> + intel_output->ddc_bus = intel_i2c_create(dev, GPIOE, (const char *)ch);
> + }
Same comment as above.
>
> if (intel_output->ddc_bus == NULL)
> goto err_i2c;
> @@ -1923,7 +1933,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 (sdvo_output_flags == MAX_SDVO_OUTPUTS_BITS)
> + sdvo_output_flags = sdvo_priv->caps.output_flags;
> +
> + if (sdvo_priv->caps.output_flags & sdvo_output_flags &
> (SDVO_OUTPUT_TMDS0 | SDVO_OUTPUT_TMDS1)) {
> if (sdvo_priv->caps.output_flags & SDVO_OUTPUT_TMDS0)
> sdvo_priv->controlled_output = SDVO_OUTPUT_TMDS0;
> @@ -1943,43 +1956,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 &
> + sdvo_output_flags & SDVO_OUTPUT_SVID0) {
> +
Since sdvo_priv->caps.output_flags is always masked with
sdvo_output_flags, it would be more clear to create a new variable with
the masked value:
const u16 maked_output_flags = sdvo_priv->caps.output_flags &
sdvo_output_flags;
Then use masked_output_flags in the tests.
> 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 &
> + sdvo_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 (sdvo_priv->caps.output_flags &
> + sdvo_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 (sdvo_priv->caps.output_flags &
> + sdvo_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 (sdvo_priv->caps.output_flags &
> + sdvo_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;
> @@ -2034,7 +2045,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)
> @@ -2044,5 +2055,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;
> +
> + for (sdvo_output_id = 1; sdvo_output_flags > 0; sdvo_output_id++) {
> +
> + sdvo_output_flags =
> + intel_sdvo_outputs_init(dev, output_device,
> + sdvo_output_id,
> + sdvo_output_flags);
> + if (sdvo_output_flags >= 0)
> + ret = true;
> + }
What if intel_sdvo_outputs_init never returns success? It would be bad
to have an infinite loop in the kernel. :)
> +
> + return ret;
> }
> +
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iEYEARECAAYFAkpI7TIACgkQX1gOwKyEAw9qEQCghPziBp61R/wT99I8MdnwT2L6
qXcAnA3dHroJOJdgtczxIiVHJmPBHkwa
=dDrZ
-----END PGP SIGNATURE-----
More information about the Intel-gfx
mailing list