[Intel-gfx] [PATCH v2 5/6] drm/i915/sdvo: Rework DDC bus handling
Ville Syrjala
ville.syrjala at linux.intel.com
Fri Aug 25 13:44:30 UTC 2023
From: Ville Syrjälä <ville.syrjala at linux.intel.com>
Each SDVO device can have up to three sets of DDC pins.
Currently we just register a single i2c_adapter for the
entire SDVO device and semi-randomly pick the "correct"
set of DDC pins during intel_sdvo_tmds_sink_detect().
This doesn't make any real sense especially if we have
multiple outputs each with their own dedicated DDC bus.
Let's clean up this mess and register a dedicated
i2c_adapter for each of the possible pin pairs. Each
output (ie. connector) can then pick the correct i2c_adapter
to use for its DDC bus. And we can just switch over to
drm_connector_init_with_ddc() to take care of the
connector->ddc association, which also populates the
"ddc" sysfs symlink as a bonus.
And now that things are based on the actual connector we can
also nuke the sketchy sdvo->controller_output thing.
Reviewed-by: Jani Nikula <jani.nikula at intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
drivers/gpu/drm/i915/display/intel_sdvo.c | 205 ++++++++++++----------
1 file changed, 114 insertions(+), 91 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 50a180c1a74b..d77a56460019 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -65,6 +65,8 @@
#define IS_TV_OR_LVDS(c) ((c)->output_flag & (SDVO_TV_MASK | SDVO_LVDS_MASK))
#define IS_DIGITAL(c) ((c)->output_flag & (SDVO_TMDS_MASK | SDVO_LVDS_MASK))
+#define HAS_DDC(c) ((c)->output_flag & (SDVO_RGB_MASK | SDVO_TMDS_MASK | \
+ SDVO_LVDS_MASK))
static const char * const tv_format_names[] = {
"NTSC_M" , "NTSC_J" , "NTSC_443",
@@ -78,20 +80,25 @@ static const char * const tv_format_names[] = {
#define TV_FORMAT_NUM ARRAY_SIZE(tv_format_names)
+struct intel_sdvo;
+
+struct intel_sdvo_ddc {
+ struct i2c_adapter ddc;
+ struct intel_sdvo *sdvo;
+ u8 ddc_bus;
+};
+
struct intel_sdvo {
struct intel_encoder base;
struct i2c_adapter *i2c;
u8 slave_addr;
- struct i2c_adapter ddc;
+ struct intel_sdvo_ddc ddc[3];
/* Register for the SDVO device: SDVOB or SDVOC */
i915_reg_t sdvo_reg;
- /* Active outputs controlled by this SDVO output */
- u16 controlled_output;
-
/*
* Capabilities of the SDVO device returned by
* intel_sdvo_get_capabilities()
@@ -108,9 +115,6 @@ struct intel_sdvo {
*/
u16 hotplug_active;
- /* DDC bus used by this SDVO encoder */
- u8 ddc_bus;
-
/*
* the sdvo flag gets lost in round trip: dtd->adjusted_mode->dtd
*/
@@ -2035,18 +2039,15 @@ intel_sdvo_hotplug(struct intel_encoder *encoder,
return intel_encoder_hotplug(encoder, connector);
}
-static bool
-intel_sdvo_multifunc_encoder(struct intel_sdvo *intel_sdvo)
-{
- /* Is there more than one type of output? */
- return hweight16(intel_sdvo->caps.output_flags) > 1;
-}
-
static const struct drm_edid *
intel_sdvo_get_edid(struct drm_connector *connector)
{
- struct intel_sdvo *sdvo = intel_attached_sdvo(to_intel_connector(connector));
- return drm_edid_read_ddc(connector, &sdvo->ddc);
+ struct i2c_adapter *ddc = connector->ddc;
+
+ if (!ddc)
+ return NULL;
+
+ return drm_edid_read_ddc(connector, ddc);
}
/* Mac mini hack -- use the same DDC as the analog connector */
@@ -2054,43 +2055,23 @@ static const struct drm_edid *
intel_sdvo_get_analog_edid(struct drm_connector *connector)
{
struct drm_i915_private *i915 = to_i915(connector->dev);
- struct i2c_adapter *i2c;
+ struct i2c_adapter *ddc;
- i2c = intel_gmbus_get_adapter(i915, i915->display.vbt.crt_ddc_pin);
+ ddc = intel_gmbus_get_adapter(i915, i915->display.vbt.crt_ddc_pin);
+ if (!ddc)
+ return NULL;
- return drm_edid_read_ddc(connector, i2c);
+ return drm_edid_read_ddc(connector, ddc);
}
static enum drm_connector_status
intel_sdvo_tmds_sink_detect(struct drm_connector *connector)
{
- struct intel_sdvo *intel_sdvo = intel_attached_sdvo(to_intel_connector(connector));
enum drm_connector_status status;
const struct drm_edid *drm_edid;
drm_edid = intel_sdvo_get_edid(connector);
- if (!drm_edid && intel_sdvo_multifunc_encoder(intel_sdvo)) {
- u8 ddc, saved_ddc = intel_sdvo->ddc_bus;
-
- /*
- * Don't use the 1 as the argument of DDC bus switch to get
- * the EDID. It is used for SDVO SPD ROM.
- */
- for (ddc = intel_sdvo->ddc_bus >> 1; ddc > 1; ddc >>= 1) {
- intel_sdvo->ddc_bus = ddc;
- drm_edid = intel_sdvo_get_edid(connector);
- if (drm_edid)
- break;
- }
- /*
- * If we found the EDID on the other bus,
- * assume that is the correct DDC bus.
- */
- if (!drm_edid)
- intel_sdvo->ddc_bus = saved_ddc;
- }
-
/*
* When there is no edid and no monitor is connected with VGA
* port, try to use the CRT ddc to read the EDID for DVI-connector.
@@ -2524,29 +2505,37 @@ static const struct drm_connector_helper_funcs intel_sdvo_connector_helper_funcs
.atomic_check = intel_sdvo_atomic_check,
};
-static void intel_sdvo_enc_destroy(struct drm_encoder *encoder)
+static void intel_sdvo_encoder_destroy(struct drm_encoder *_encoder)
{
- struct intel_sdvo *intel_sdvo = to_sdvo(to_intel_encoder(encoder));
+ struct intel_encoder *encoder = to_intel_encoder(_encoder);
+ struct intel_sdvo *sdvo = to_sdvo(encoder);
+ int i;
- i2c_del_adapter(&intel_sdvo->ddc);
- intel_encoder_destroy(encoder);
-}
+ for (i = 0; i < ARRAY_SIZE(sdvo->ddc); i++) {
+ if (sdvo->ddc[i].ddc_bus)
+ i2c_del_adapter(&sdvo->ddc[i].ddc);
+ }
+
+ drm_encoder_cleanup(&encoder->base);
+ kfree(sdvo);
+};
static const struct drm_encoder_funcs intel_sdvo_enc_funcs = {
- .destroy = intel_sdvo_enc_destroy,
+ .destroy = intel_sdvo_encoder_destroy,
};
-static void
-intel_sdvo_guess_ddc_bus(struct intel_sdvo *sdvo)
+static int
+intel_sdvo_guess_ddc_bus(struct intel_sdvo *sdvo,
+ struct intel_sdvo_connector *connector)
{
u16 mask = 0;
- unsigned int num_bits;
+ int num_bits;
/*
* Make a mask of outputs less than or equal to our own priority in the
* list.
*/
- switch (sdvo->controlled_output) {
+ switch (connector->output_flag) {
case SDVO_OUTPUT_LVDS1:
mask |= SDVO_OUTPUT_LVDS1;
fallthrough;
@@ -2575,7 +2564,7 @@ intel_sdvo_guess_ddc_bus(struct intel_sdvo *sdvo)
num_bits = 3;
/* Corresponds to SDVO_CONTROL_BUS_DDCx */
- sdvo->ddc_bus = 1 << num_bits;
+ return num_bits;
}
/*
@@ -2585,11 +2574,13 @@ intel_sdvo_guess_ddc_bus(struct intel_sdvo *sdvo)
* DDC bus number assignment is in a priority order of RGB outputs, then TMDS
* outputs, then LVDS outputs.
*/
-static void
-intel_sdvo_select_ddc_bus(struct intel_sdvo *sdvo)
+static struct intel_sdvo_ddc *
+intel_sdvo_select_ddc_bus(struct intel_sdvo *sdvo,
+ struct intel_sdvo_connector *connector)
{
struct drm_i915_private *dev_priv = to_i915(sdvo->base.base.dev);
struct sdvo_device_mapping *mapping;
+ int ddc_bus;
if (sdvo->base.port == PORT_B)
mapping = &dev_priv->display.vbt.sdvo_mappings[0];
@@ -2597,9 +2588,14 @@ intel_sdvo_select_ddc_bus(struct intel_sdvo *sdvo)
mapping = &dev_priv->display.vbt.sdvo_mappings[1];
if (mapping->initialized)
- sdvo->ddc_bus = 1 << ((mapping->ddc_pin & 0xf0) >> 4);
+ ddc_bus = (mapping->ddc_pin & 0xf0) >> 4;
else
- intel_sdvo_guess_ddc_bus(sdvo);
+ ddc_bus = intel_sdvo_guess_ddc_bus(sdvo, connector);
+
+ if (ddc_bus < 1 || ddc_bus > 3)
+ return NULL;
+
+ return &sdvo->ddc[ddc_bus - 1];
}
static void
@@ -2682,22 +2678,30 @@ intel_sdvo_get_slave_addr(struct intel_sdvo *sdvo)
return 0x72;
}
+static int
+intel_sdvo_init_ddc_proxy(struct intel_sdvo_ddc *ddc,
+ struct intel_sdvo *sdvo, int bit);
+
static int
intel_sdvo_connector_init(struct intel_sdvo_connector *connector,
struct intel_sdvo *encoder)
{
- struct drm_connector *drm_connector;
+ struct drm_i915_private *i915 = to_i915(encoder->base.base.dev);
+ struct intel_sdvo_ddc *ddc = NULL;
int ret;
- drm_connector = &connector->base.base;
- ret = drm_connector_init(encoder->base.base.dev,
- drm_connector,
- &intel_sdvo_connector_funcs,
- connector->base.base.connector_type);
+ if (HAS_DDC(connector))
+ ddc = intel_sdvo_select_ddc_bus(encoder, connector);
+
+ ret = drm_connector_init_with_ddc(encoder->base.base.dev,
+ &connector->base.base,
+ &intel_sdvo_connector_funcs,
+ connector->base.base.connector_type,
+ ddc ? &ddc->ddc : NULL);
if (ret < 0)
return ret;
- drm_connector_helper_add(drm_connector,
+ drm_connector_helper_add(&connector->base.base,
&intel_sdvo_connector_helper_funcs);
connector->base.base.display_info.subpixel_order = SubPixelHorizontalRGB;
@@ -2706,6 +2710,11 @@ intel_sdvo_connector_init(struct intel_sdvo_connector *connector,
intel_connector_attach_encoder(&connector->base, &encoder->base);
+ if (ddc)
+ drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] using %s\n",
+ connector->base.base.base.id, connector->base.base.name,
+ ddc->ddc.name);
+
return 0;
}
@@ -2903,7 +2912,7 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, u16 type)
if (!intel_panel_preferred_fixed_mode(intel_connector)) {
mutex_lock(&i915->drm.mode_config.mutex);
- intel_ddc_get_modes(connector, &intel_sdvo->ddc);
+ intel_ddc_get_modes(connector, connector->ddc);
intel_panel_add_edid_fixed_modes(intel_connector, false);
mutex_unlock(&i915->drm.mode_config.mutex);
@@ -2978,10 +2987,6 @@ intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo)
return false;
}
- intel_sdvo->controlled_output = flags;
-
- intel_sdvo_select_ddc_bus(intel_sdvo);
-
for (i = 0; i < ARRAY_SIZE(probe_order); i++) {
u16 type = flags & probe_order[i];
@@ -3234,9 +3239,10 @@ static int intel_sdvo_ddc_proxy_xfer(struct i2c_adapter *adapter,
struct i2c_msg *msgs,
int num)
{
- struct intel_sdvo *sdvo = adapter->algo_data;
+ struct intel_sdvo_ddc *ddc = adapter->algo_data;
+ struct intel_sdvo *sdvo = ddc->sdvo;
- if (!__intel_sdvo_set_control_bus_switch(sdvo, sdvo->ddc_bus))
+ if (!__intel_sdvo_set_control_bus_switch(sdvo, 1 << ddc->ddc_bus))
return -EIO;
return sdvo->i2c->algo->master_xfer(sdvo->i2c, msgs, num);
@@ -3244,7 +3250,9 @@ static int intel_sdvo_ddc_proxy_xfer(struct i2c_adapter *adapter,
static u32 intel_sdvo_ddc_proxy_func(struct i2c_adapter *adapter)
{
- struct intel_sdvo *sdvo = adapter->algo_data;
+ struct intel_sdvo_ddc *ddc = adapter->algo_data;
+ struct intel_sdvo *sdvo = ddc->sdvo;
+
return sdvo->i2c->algo->functionality(sdvo->i2c);
}
@@ -3256,21 +3264,27 @@ static const struct i2c_algorithm intel_sdvo_ddc_proxy = {
static void proxy_lock_bus(struct i2c_adapter *adapter,
unsigned int flags)
{
- struct intel_sdvo *sdvo = adapter->algo_data;
+ struct intel_sdvo_ddc *ddc = adapter->algo_data;
+ struct intel_sdvo *sdvo = ddc->sdvo;
+
sdvo->i2c->lock_ops->lock_bus(sdvo->i2c, flags);
}
static int proxy_trylock_bus(struct i2c_adapter *adapter,
unsigned int flags)
{
- struct intel_sdvo *sdvo = adapter->algo_data;
+ struct intel_sdvo_ddc *ddc = adapter->algo_data;
+ struct intel_sdvo *sdvo = ddc->sdvo;
+
return sdvo->i2c->lock_ops->trylock_bus(sdvo->i2c, flags);
}
static void proxy_unlock_bus(struct i2c_adapter *adapter,
unsigned int flags)
{
- struct intel_sdvo *sdvo = adapter->algo_data;
+ struct intel_sdvo_ddc *ddc = adapter->algo_data;
+ struct intel_sdvo *sdvo = ddc->sdvo;
+
sdvo->i2c->lock_ops->unlock_bus(sdvo->i2c, flags);
}
@@ -3280,21 +3294,26 @@ static const struct i2c_lock_operations proxy_lock_ops = {
.unlock_bus = proxy_unlock_bus,
};
-static bool
-intel_sdvo_init_ddc_proxy(struct intel_sdvo *sdvo)
+static int
+intel_sdvo_init_ddc_proxy(struct intel_sdvo_ddc *ddc,
+ struct intel_sdvo *sdvo, int ddc_bus)
{
struct drm_i915_private *dev_priv = to_i915(sdvo->base.base.dev);
struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
- sdvo->ddc.owner = THIS_MODULE;
- sdvo->ddc.class = I2C_CLASS_DDC;
- snprintf(sdvo->ddc.name, I2C_NAME_SIZE, "SDVO DDC proxy");
- sdvo->ddc.dev.parent = &pdev->dev;
- sdvo->ddc.algo_data = sdvo;
- sdvo->ddc.algo = &intel_sdvo_ddc_proxy;
- sdvo->ddc.lock_ops = &proxy_lock_ops;
+ ddc->sdvo = sdvo;
+ ddc->ddc_bus = ddc_bus;
- return i2c_add_adapter(&sdvo->ddc) == 0;
+ ddc->ddc.owner = THIS_MODULE;
+ ddc->ddc.class = I2C_CLASS_DDC;
+ snprintf(ddc->ddc.name, I2C_NAME_SIZE, "SDVO %c DDC%d",
+ port_name(sdvo->base.port), ddc_bus);
+ ddc->ddc.dev.parent = &pdev->dev;
+ ddc->ddc.algo_data = ddc;
+ ddc->ddc.algo = &intel_sdvo_ddc_proxy;
+ ddc->ddc.lock_ops = &proxy_lock_ops;
+
+ return i2c_add_adapter(&ddc->ddc);
}
static bool is_sdvo_port_valid(struct drm_i915_private *dev_priv, enum port port)
@@ -3341,9 +3360,8 @@ bool intel_sdvo_init(struct drm_i915_private *dev_priv,
intel_sdvo->sdvo_reg = sdvo_reg;
intel_sdvo->slave_addr = intel_sdvo_get_slave_addr(intel_sdvo) >> 1;
+
intel_sdvo_select_i2c_bus(intel_sdvo);
- if (!intel_sdvo_init_ddc_proxy(intel_sdvo))
- goto err_i2c_bus;
/* Read the regs to test if we can talk to the device */
for (i = 0; i < 0x40; i++) {
@@ -3376,6 +3394,15 @@ bool intel_sdvo_init(struct drm_i915_private *dev_priv,
intel_sdvo->colorimetry_cap =
intel_sdvo_get_colorimetry_cap(intel_sdvo);
+ for (i = 0; i < ARRAY_SIZE(intel_sdvo->ddc); i++) {
+ int ret;
+
+ ret = intel_sdvo_init_ddc_proxy(&intel_sdvo->ddc[i],
+ intel_sdvo, i + 1);
+ if (ret)
+ goto err;
+ }
+
if (!intel_sdvo_output_setup(intel_sdvo)) {
drm_dbg_kms(&dev_priv->drm,
"SDVO output failed to setup on %s\n",
@@ -3436,13 +3463,9 @@ bool intel_sdvo_init(struct drm_i915_private *dev_priv,
err_output:
intel_sdvo_output_cleanup(intel_sdvo);
-
err:
- i2c_del_adapter(&intel_sdvo->ddc);
-err_i2c_bus:
intel_sdvo_unselect_i2c_bus(intel_sdvo);
- drm_encoder_cleanup(&intel_encoder->base);
- kfree(intel_sdvo);
+ intel_sdvo_encoder_destroy(&intel_encoder->base);
return false;
}
--
2.41.0
More information about the Intel-gfx
mailing list