We were recently bitten by drm_edid_to_eld() clearing the connector type, and us failing to set it back for DP. Here's a few ELD related patches to try to unify ELD handling and make it a bit simpler for drivers to get it right.
Apologies for the massive Cc list; it's the maintainers of all drivers that call drm_edid_to_eld().
I'm open to splitting up patch 6 to driver specific patches as needed, but I'd think it would be just fine to merge via drm-misc as-is too.
BR, Jani.
Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Archit Taneja architt@codeaurora.org Cc: Andrzej Hajda a.hajda@samsung.com Cc: Russell King linux@armlinux.org.uk Cc: CK Hu ck.hu@mediatek.com Cc: Philipp Zabel p.zabel@pengutronix.de Cc: Ben Skeggs bskeggs@redhat.com Cc: Mark Yao mark.yao@rock-chips.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Cc: Thierry Reding thierry.reding@gmail.com Cc: Eric Anholt eric@anholt.net
Jani Nikula (7): drm/edid: use macros for ELD offsets and values drm/edid: set ELD connector type in drm_edid_to_eld() drm/i915: remove redundant ELD connector type update drm/edid: abstract connector ELD clearing drm/edid: build ELD in drm_add_edid_modes() drm/drivers: drop redundant drm_edid_to_eld() calls drm/edid: make drm_edid_to_eld() static
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 1 - drivers/gpu/drm/bridge/analogix-anx78xx.c | 2 - drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 - drivers/gpu/drm/drm_edid.c | 70 +++++++++++++++----------- drivers/gpu/drm/i2c/tda998x_drv.c | 1 - drivers/gpu/drm/i915/intel_dp.c | 1 - drivers/gpu/drm/i915/intel_modes.c | 18 ------- drivers/gpu/drm/mediatek/mtk_hdmi.c | 1 - drivers/gpu/drm/nouveau/nv50_display.c | 5 +- drivers/gpu/drm/radeon/radeon_connectors.c | 1 - drivers/gpu/drm/radeon/radeon_dp_mst.c | 1 - drivers/gpu/drm/rockchip/cdn-dp-core.c | 4 +- drivers/gpu/drm/sti/sti_hdmi.c | 1 - drivers/gpu/drm/tegra/output.c | 1 - drivers/gpu/drm/vc4/vc4_hdmi.c | 1 - include/drm/drm_edid.h | 1 - include/drm/drm_modeset_helper_vtables.h | 3 -- 17 files changed, 44 insertions(+), 70 deletions(-)
We have the macros, use them. No functional changes.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 00ddabfbf980..6229735ecc15 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3756,8 +3756,8 @@ drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) { u8 len = cea_db_payload_len(db);
- if (len >= 6) - connector->eld[5] |= (db[6] >> 7) << 1; /* Supports_AI */ + if (len >= 6 && (db[6] & (1 << 7))) + connector->eld[DRM_ELD_SAD_COUNT_CONN_TYPE] |= DRM_ELD_SUPPORTS_AI; if (len >= 8) { connector->latency_present[0] = db[8] >> 7; connector->latency_present[1] = (db[8] >> 6) & 1; @@ -3865,17 +3865,18 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) return; }
- mnl = get_monitor_name(edid, eld + 20); + mnl = get_monitor_name(edid, &eld[DRM_ELD_MONITOR_NAME_STRING]); + DRM_DEBUG_KMS("ELD monitor %s\n", &eld[DRM_ELD_MONITOR_NAME_STRING]);
- eld[4] = (cea[1] << 5) | mnl; - DRM_DEBUG_KMS("ELD monitor %s\n", eld + 20); + eld[DRM_ELD_CEA_EDID_VER_MNL] = cea[1] << DRM_ELD_CEA_EDID_VER_SHIFT; + eld[DRM_ELD_CEA_EDID_VER_MNL] |= mnl;
- eld[0] = 2 << 3; /* ELD version: 2 */ + eld[DRM_ELD_VER] = DRM_ELD_VER_CEA861D;
- eld[16] = edid->mfg_id[0]; - eld[17] = edid->mfg_id[1]; - eld[18] = edid->prod_code[0]; - eld[19] = edid->prod_code[1]; + eld[DRM_ELD_MANUFACTURER_NAME0] = edid->mfg_id[0]; + eld[DRM_ELD_MANUFACTURER_NAME1] = edid->mfg_id[1]; + eld[DRM_ELD_PRODUCT_CODE0] = edid->prod_code[0]; + eld[DRM_ELD_PRODUCT_CODE1] = edid->prod_code[1];
if (cea_revision(cea) >= 3) { int i, start, end; @@ -3896,14 +3897,14 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) /* Audio Data Block, contains SADs */ sad_count = min(dbl / 3, 15 - total_sad_count); if (sad_count >= 1) - memcpy(eld + 20 + mnl + total_sad_count * 3, + memcpy(&eld[DRM_ELD_CEA_SAD(mnl, total_sad_count)], &db[1], sad_count * 3); total_sad_count += sad_count; break; case SPEAKER_BLOCK: /* Speaker Allocation Data Block */ if (dbl >= 1) - eld[7] = db[1]; + eld[DRM_ELD_SPEAKER] = db[1]; break; case VENDOR_BLOCK: /* HDMI Vendor-Specific Data Block */ @@ -3915,7 +3916,7 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) } } } - eld[5] |= total_sad_count << 4; + eld[DRM_ELD_SAD_COUNT_CONN_TYPE] |= total_sad_count << DRM_ELD_SAD_COUNT_SHIFT;
eld[DRM_ELD_BASELINE_ELD_LEN] = DIV_ROUND_UP(drm_eld_calc_baseline_block_size(eld), 4);
Since drm_edid_to_eld() knows the connector type, we can set the type in ELD while at it. Most connectors this gets called on are not DP encoders, and with the HDMI type being 0, this does not change behaviour for non-DP.
For i915 having this in place earlier would have saved a considerable amount of debugging that lead to the fix 2d8f63297b9f ("drm/i915: always update ELD connector type after get modes"). I don't see other drivers, even the ones calling drm_edid_to_eld() on DP connectors, setting the connector type in ELD.
Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Archit Taneja architt@codeaurora.org Cc: Andrzej Hajda a.hajda@samsung.com Cc: Russell King linux@armlinux.org.uk Cc: CK Hu ck.hu@mediatek.com Cc: Philipp Zabel p.zabel@pengutronix.de Cc: Ben Skeggs bskeggs@redhat.com Cc: Mark Yao mark.yao@rock-chips.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Cc: Thierry Reding thierry.reding@gmail.com Cc: Eric Anholt eric@anholt.net Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 6229735ecc15..3139c2e90e32 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3835,8 +3835,7 @@ EXPORT_SYMBOL(drm_edid_get_monitor_name); * @edid: EDID to parse * * Fill the ELD (EDID-Like Data) buffer for passing to the audio driver. The - * Conn_Type, HDCP and Port_ID ELD fields are left for the graphics driver to - * fill in. + * HDCP and Port_ID ELD fields are left for the graphics driver to fill in. */ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) { @@ -3918,6 +3917,12 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) } eld[DRM_ELD_SAD_COUNT_CONN_TYPE] |= total_sad_count << DRM_ELD_SAD_COUNT_SHIFT;
+ if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort || + connector->connector_type == DRM_MODE_CONNECTOR_eDP) + eld[DRM_ELD_SAD_COUNT_CONN_TYPE] |= DRM_ELD_CONN_TYPE_DP; + else + eld[DRM_ELD_SAD_COUNT_CONN_TYPE] |= DRM_ELD_CONN_TYPE_HDMI; + eld[DRM_ELD_BASELINE_ELD_LEN] = DIV_ROUND_UP(drm_eld_calc_baseline_block_size(eld), 4);
drm_edid_to_eld() now sets ELD connector type, remove the redundant update.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/i915/intel_modes.c | 17 ----------------- 1 file changed, 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c index 28a778b785ac..951e834dd274 100644 --- a/drivers/gpu/drm/i915/intel_modes.c +++ b/drivers/gpu/drm/i915/intel_modes.c @@ -30,21 +30,6 @@ #include "intel_drv.h" #include "i915_drv.h"
-static void intel_connector_update_eld_conn_type(struct drm_connector *connector) -{ - u8 conn_type; - - if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort || - connector->connector_type == DRM_MODE_CONNECTOR_eDP) { - conn_type = DRM_ELD_CONN_TYPE_DP; - } else { - conn_type = DRM_ELD_CONN_TYPE_HDMI; - } - - connector->eld[DRM_ELD_SAD_COUNT_CONN_TYPE] &= ~DRM_ELD_CONN_TYPE_MASK; - connector->eld[DRM_ELD_SAD_COUNT_CONN_TYPE] |= conn_type; -} - /** * intel_connector_update_modes - update connector from edid * @connector: DRM connector device to use @@ -59,8 +44,6 @@ int intel_connector_update_modes(struct drm_connector *connector, ret = drm_add_edid_modes(connector, edid); drm_edid_to_eld(connector, edid);
- intel_connector_update_eld_conn_type(connector); - return ret; }
Preparation for future work. No functional changes.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 3139c2e90e32..3162ea58e450 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3829,6 +3829,18 @@ void drm_edid_get_monitor_name(struct edid *edid, char *name, int bufsize) } EXPORT_SYMBOL(drm_edid_get_monitor_name);
+static void clear_eld(struct drm_connector *connector) +{ + memset(connector->eld, 0, sizeof(connector->eld)); + + connector->latency_present[0] = false; + connector->latency_present[1] = false; + connector->video_latency[0] = 0; + connector->audio_latency[0] = 0; + connector->video_latency[1] = 0; + connector->audio_latency[1] = 0; +} + /** * drm_edid_to_eld - build ELD from EDID * @connector: connector corresponding to the HDMI/DP sink @@ -3846,14 +3858,7 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) int mnl; int dbl;
- memset(eld, 0, sizeof(connector->eld)); - - connector->latency_present[0] = false; - connector->latency_present[1] = false; - connector->video_latency[0] = 0; - connector->audio_latency[0] = 0; - connector->video_latency[1] = 0; - connector->audio_latency[1] = 0; + clear_eld(connector);
if (!edid) return;
Call drm_edid_to_eld() from drm_add_edid_modes() to fill in the ELD automatically. There's no harm in doing this for connectors that do not support audio.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 3162ea58e450..4e3ef3d91b95 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4612,8 +4612,8 @@ static int add_displayid_detailed_modes(struct drm_connector *connector, * @edid: EDID data * * Add the specified modes to the connector's mode list. Also fills out the - * &drm_display_info structure in @connector with any information which can be - * derived from the edid. + * &drm_display_info structure and ELD in @connector with any information which + * can be derived from the edid. * * Return: The number of modes added or 0 if we couldn't find any. */ @@ -4623,9 +4623,11 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) u32 quirks;
if (edid == NULL) { + clear_eld(connector); return 0; } if (!drm_edid_is_valid(edid)) { + clear_eld(connector); dev_warn(connector->dev->dev, "%s: EDID invalid.\n", connector->name); return 0; @@ -4633,6 +4635,8 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
quirks = edid_get_quirks(edid);
+ drm_edid_to_eld(connector, edid); + /* * CEA-861-F adds ycbcr capability map block, for HDMI 2.0 sinks. * To avoid multiple parsing of same block, lets parse that map
drm_add_edid_modes() now fills in the ELD automatically, so the calls to drm_edid_to_eld() are redundant. Remove them.
All the other places are obvious, but nv50 has detached drm_edid_to_eld() from the drm_add_edid_modes() call.
Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Archit Taneja architt@codeaurora.org Cc: Andrzej Hajda a.hajda@samsung.com Cc: Russell King linux@armlinux.org.uk Cc: CK Hu ck.hu@mediatek.com Cc: Philipp Zabel p.zabel@pengutronix.de Cc: Ben Skeggs bskeggs@redhat.com Cc: Mark Yao mark.yao@rock-chips.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Cc: Thierry Reding thierry.reding@gmail.com Cc: Eric Anholt eric@anholt.net Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 1 - drivers/gpu/drm/bridge/analogix-anx78xx.c | 2 -- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 -- drivers/gpu/drm/i2c/tda998x_drv.c | 1 - drivers/gpu/drm/i915/intel_dp.c | 1 - drivers/gpu/drm/i915/intel_modes.c | 1 - drivers/gpu/drm/mediatek/mtk_hdmi.c | 1 - drivers/gpu/drm/nouveau/nv50_display.c | 5 +---- drivers/gpu/drm/radeon/radeon_connectors.c | 1 - drivers/gpu/drm/radeon/radeon_dp_mst.c | 1 - drivers/gpu/drm/rockchip/cdn-dp-core.c | 4 +--- drivers/gpu/drm/sti/sti_hdmi.c | 1 - drivers/gpu/drm/tegra/output.c | 1 - drivers/gpu/drm/vc4/vc4_hdmi.c | 1 - 14 files changed, 2 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c index df9cbc78e168..8ca3783f2deb 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c @@ -358,7 +358,6 @@ static int amdgpu_connector_ddc_get_modes(struct drm_connector *connector) if (amdgpu_connector->edid) { drm_mode_connector_update_edid_property(connector, amdgpu_connector->edid); ret = drm_add_edid_modes(connector, amdgpu_connector->edid); - drm_edid_to_eld(connector, amdgpu_connector->edid); return ret; } drm_mode_connector_update_edid_property(connector, NULL); diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c index 9385eb0b1ee4..ed12a7ddd64a 100644 --- a/drivers/gpu/drm/bridge/analogix-anx78xx.c +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c @@ -977,8 +977,6 @@ static int anx78xx_get_modes(struct drm_connector *connector) }
num_modes = drm_add_edid_modes(connector, anx78xx->edid); - /* Store the ELD */ - drm_edid_to_eld(connector, anx78xx->edid);
unlock: mutex_unlock(&anx78xx->lock); diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index bf14214fa464..a64ce7112288 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -1910,8 +1910,6 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector) drm_mode_connector_update_edid_property(connector, edid); cec_notifier_set_phys_addr_from_edid(hdmi->cec_notifier, edid); ret = drm_add_edid_modes(connector, edid); - /* Store the ELD */ - drm_edid_to_eld(connector, edid); kfree(edid); } else { dev_dbg(hdmi->dev, "failed to get edid\n"); diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 4d1f45acf2cd..60981505763c 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -1100,7 +1100,6 @@ static int tda998x_connector_get_modes(struct drm_connector *connector)
drm_mode_connector_update_edid_property(connector, edid); n = drm_add_edid_modes(connector, edid); - drm_edid_to_eld(connector, edid);
kfree(edid);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index d27c0145ac91..cddd96b24878 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -5889,7 +5889,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, if (drm_add_edid_modes(connector, edid)) { drm_mode_connector_update_edid_property(connector, edid); - drm_edid_to_eld(connector, edid); } else { kfree(edid); edid = ERR_PTR(-EINVAL); diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c index 951e834dd274..b39846613e3c 100644 --- a/drivers/gpu/drm/i915/intel_modes.c +++ b/drivers/gpu/drm/i915/intel_modes.c @@ -42,7 +42,6 @@ int intel_connector_update_modes(struct drm_connector *connector,
drm_mode_connector_update_edid_property(connector, edid); ret = drm_add_edid_modes(connector, edid); - drm_edid_to_eld(connector, edid);
return ret; } diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c index 3ff502771ba2..b78791061983 100644 --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c @@ -1222,7 +1222,6 @@ static int mtk_hdmi_conn_get_modes(struct drm_connector *conn) drm_mode_connector_update_edid_property(conn, edid);
ret = drm_add_edid_modes(conn, edid); - drm_edid_to_eld(conn, edid); kfree(edid); return ret; } diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c index e4751f92b342..e0a190a0f029 100644 --- a/drivers/gpu/drm/nouveau/nv50_display.c +++ b/drivers/gpu/drm/nouveau/nv50_display.c @@ -2688,7 +2688,6 @@ nv50_audio_enable(struct drm_encoder *encoder, struct drm_display_mode *mode) if (!drm_detect_monitor_audio(nv_connector->edid)) return;
- drm_edid_to_eld(&nv_connector->base, nv_connector->edid); memcpy(args.data, nv_connector->base.eld, sizeof(args.data));
nvif_mthd(disp->disp, 0, &args, @@ -3064,10 +3063,8 @@ nv50_mstc_get_modes(struct drm_connector *connector)
mstc->edid = drm_dp_mst_get_edid(&mstc->connector, mstc->port->mgr, mstc->port); drm_mode_connector_update_edid_property(&mstc->connector, mstc->edid); - if (mstc->edid) { + if (mstc->edid) ret = drm_add_edid_modes(&mstc->connector, mstc->edid); - drm_edid_to_eld(&mstc->connector, mstc->edid); - }
if (!mstc->connector.display_info.bpc) mstc->connector.display_info.bpc = 8; diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c index 59dcefb2df3b..5012f5e47a1e 100644 --- a/drivers/gpu/drm/radeon/radeon_connectors.c +++ b/drivers/gpu/drm/radeon/radeon_connectors.c @@ -392,7 +392,6 @@ static int radeon_ddc_get_modes(struct drm_connector *connector) if (radeon_connector->edid) { drm_mode_connector_update_edid_property(connector, radeon_connector->edid); ret = drm_add_edid_modes(connector, radeon_connector->edid); - drm_edid_to_eld(connector, radeon_connector->edid); return ret; } drm_mode_connector_update_edid_property(connector, NULL); diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c index ebdf1b859cb6..1768926d2ebc 100644 --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c @@ -196,7 +196,6 @@ static int radeon_dp_mst_get_ddc_modes(struct drm_connector *connector) if (radeon_connector->edid) { drm_mode_connector_update_edid_property(&radeon_connector->base, radeon_connector->edid); ret = drm_add_edid_modes(&radeon_connector->base, radeon_connector->edid); - drm_edid_to_eld(&radeon_connector->base, radeon_connector->edid); return ret; } drm_mode_connector_update_edid_property(&radeon_connector->base, NULL); diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c index 275844d0d0ec..ec999d9f15f6 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c @@ -276,11 +276,9 @@ static int cdn_dp_connector_get_modes(struct drm_connector *connector)
dp->sink_has_audio = drm_detect_monitor_audio(edid); ret = drm_add_edid_modes(connector, edid); - if (ret) { + if (ret) drm_mode_connector_update_edid_property(connector, edid); - drm_edid_to_eld(connector, edid); - } } mutex_unlock(&dp->lock);
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c index 30f02d2fdd03..d1902750a85d 100644 --- a/drivers/gpu/drm/sti/sti_hdmi.c +++ b/drivers/gpu/drm/sti/sti_hdmi.c @@ -976,7 +976,6 @@ static int sti_hdmi_connector_get_modes(struct drm_connector *connector)
count = drm_add_edid_modes(connector, edid); drm_mode_connector_update_edid_property(connector, edid); - drm_edid_to_eld(connector, edid);
kfree(edid); return count; diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c index 1cfbacea8113..24f8a3b712b4 100644 --- a/drivers/gpu/drm/tegra/output.c +++ b/drivers/gpu/drm/tegra/output.c @@ -39,7 +39,6 @@ int tegra_output_connector_get_modes(struct drm_connector *connector)
if (edid) { err = drm_add_edid_modes(connector, edid); - drm_edid_to_eld(connector, edid); kfree(edid); }
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index fa37a1c07cf6..9a9a6b4acccf 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -287,7 +287,6 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
drm_mode_connector_update_edid_property(connector, edid); ret = drm_add_edid_modes(connector, edid); - drm_edid_to_eld(connector, edid); kfree(edid);
return ret;
On Wed, Nov 01, 2017 at 04:21:02PM +0200, Jani Nikula wrote:
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c index e4751f92b342..e0a190a0f029 100644 --- a/drivers/gpu/drm/nouveau/nv50_display.c +++ b/drivers/gpu/drm/nouveau/nv50_display.c @@ -2688,7 +2688,6 @@ nv50_audio_enable(struct drm_encoder *encoder, struct drm_display_mode *mode) if (!drm_detect_monitor_audio(nv_connector->edid)) return;
drm_edid_to_eld(&nv_connector->base, nv_connector->edid); memcpy(args.data, nv_connector->base.eld, sizeof(args.data));
nvif_mthd(disp->disp, 0, &args,
This being the only call outside a .get_modes() hook means this is the only one that might change some behaviour.
Looks like nv_connector->edid gets updated in .detect() as one might expect, so in theory we might get a mismatch between the drm_detect_monitor_audio(edid) check above and the actual eld contents here if .detect() gets called without .get_modes(). Not a problem for the probe helper's .fill_modes(), but eg. the poll helper does do that.
I guess we could always consider doing the edid_to_eld() already from .detect()/.force() for all drivers if we think there would be a good reason to populate the ELD even if .get_modes()/.fill_modes() hasn't been called.
Jani Nikula jani.nikula@intel.com writes:
drm_add_edid_modes() now fills in the ELD automatically, so the calls to drm_edid_to_eld() are redundant. Remove them.
All the other places are obvious, but nv50 has detached drm_edid_to_eld() from the drm_add_edid_modes() call.
Nice! For vc4,
Acked-by: Eric Anholt eric@anholt.net
On 11/01/2017 07:51 PM, Jani Nikula wrote:
drm_add_edid_modes() now fills in the ELD automatically, so the calls to drm_edid_to_eld() are redundant. Remove them.
All the other places are obvious, but nv50 has detached drm_edid_to_eld() from the drm_add_edid_modes() call.
For the bridge drivers:
Acked-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
This is no longer needed outside of drm_edid.c.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 5 ++--- include/drm/drm_edid.h | 1 - include/drm/drm_modeset_helper_vtables.h | 3 --- 3 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 4e3ef3d91b95..749d07a01772 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3841,7 +3841,7 @@ static void clear_eld(struct drm_connector *connector) connector->audio_latency[1] = 0; }
-/** +/* * drm_edid_to_eld - build ELD from EDID * @connector: connector corresponding to the HDMI/DP sink * @edid: EDID to parse @@ -3849,7 +3849,7 @@ static void clear_eld(struct drm_connector *connector) * Fill the ELD (EDID-Like Data) buffer for passing to the audio driver. The * HDCP and Port_ID ELD fields are left for the graphics driver to fill in. */ -void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) +static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) { uint8_t *eld = connector->eld; u8 *cea; @@ -3934,7 +3934,6 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) DRM_DEBUG_KMS("ELD size %d, SAD count %d\n", drm_eld_size(eld), total_sad_count); } -EXPORT_SYMBOL(drm_edid_to_eld);
/** * drm_edid_to_sad - extracts SADs from EDID diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 6f35909b8add..9e4e23524840 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -333,7 +333,6 @@ struct drm_encoder; struct drm_connector; struct drm_display_mode;
-void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid); int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads); int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb); int drm_av_sync_delay(struct drm_connector *connector, diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h index 16646c44b7df..3e76ca805b0f 100644 --- a/include/drm/drm_modeset_helper_vtables.h +++ b/include/drm/drm_modeset_helper_vtables.h @@ -801,9 +801,6 @@ struct drm_connector_helper_funcs { * resolution can call drm_add_modes_noedid(), and mark the preferred * one using drm_set_preferred_mode(). * - * Finally drivers that support audio probably want to update the ELD - * data, too, using drm_edid_to_eld(). - * * This function is only called after the @detect hook has indicated * that a sink is connected and when the EDID isn't overridden through * sysfs or the kernel commandline.
On Wed, Nov 01, 2017 at 04:20:56PM +0200, Jani Nikula wrote:
We were recently bitten by drm_edid_to_eld() clearing the connector type, and us failing to set it back for DP. Here's a few ELD related patches to try to unify ELD handling and make it a bit simpler for drivers to get it right.
Apologies for the massive Cc list; it's the maintainers of all drivers that call drm_edid_to_eld().
I'm open to splitting up patch 6 to driver specific patches as needed, but I'd think it would be just fine to merge via drm-misc as-is too.
Entire series lgtm so Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
BR, Jani.
Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Archit Taneja architt@codeaurora.org Cc: Andrzej Hajda a.hajda@samsung.com Cc: Russell King linux@armlinux.org.uk Cc: CK Hu ck.hu@mediatek.com Cc: Philipp Zabel p.zabel@pengutronix.de Cc: Ben Skeggs bskeggs@redhat.com Cc: Mark Yao mark.yao@rock-chips.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Cc: Thierry Reding thierry.reding@gmail.com Cc: Eric Anholt eric@anholt.net
Jani Nikula (7): drm/edid: use macros for ELD offsets and values drm/edid: set ELD connector type in drm_edid_to_eld() drm/i915: remove redundant ELD connector type update drm/edid: abstract connector ELD clearing drm/edid: build ELD in drm_add_edid_modes() drm/drivers: drop redundant drm_edid_to_eld() calls drm/edid: make drm_edid_to_eld() static
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 1 - drivers/gpu/drm/bridge/analogix-anx78xx.c | 2 - drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 - drivers/gpu/drm/drm_edid.c | 70 +++++++++++++++----------- drivers/gpu/drm/i2c/tda998x_drv.c | 1 - drivers/gpu/drm/i915/intel_dp.c | 1 - drivers/gpu/drm/i915/intel_modes.c | 18 ------- drivers/gpu/drm/mediatek/mtk_hdmi.c | 1 - drivers/gpu/drm/nouveau/nv50_display.c | 5 +- drivers/gpu/drm/radeon/radeon_connectors.c | 1 - drivers/gpu/drm/radeon/radeon_dp_mst.c | 1 - drivers/gpu/drm/rockchip/cdn-dp-core.c | 4 +- drivers/gpu/drm/sti/sti_hdmi.c | 1 - drivers/gpu/drm/tegra/output.c | 1 - drivers/gpu/drm/vc4/vc4_hdmi.c | 1 - include/drm/drm_edid.h | 1 - include/drm/drm_modeset_helper_vtables.h | 3 -- 17 files changed, 44 insertions(+), 70 deletions(-)
-- 2.11.0
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Nov 1, 2017 at 10:20 AM, Jani Nikula jani.nikula@intel.com wrote:
We were recently bitten by drm_edid_to_eld() clearing the connector type, and us failing to set it back for DP. Here's a few ELD related patches to try to unify ELD handling and make it a bit simpler for drivers to get it right.
Apologies for the massive Cc list; it's the maintainers of all drivers that call drm_edid_to_eld().
I'm open to splitting up patch 6 to driver specific patches as needed, but I'd think it would be just fine to merge via drm-misc as-is too.
Nice! Series is: Reviewed-by: Alex Deucher alexander.deucher@amd.com
BR, Jani.
Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Archit Taneja architt@codeaurora.org Cc: Andrzej Hajda a.hajda@samsung.com Cc: Russell King linux@armlinux.org.uk Cc: CK Hu ck.hu@mediatek.com Cc: Philipp Zabel p.zabel@pengutronix.de Cc: Ben Skeggs bskeggs@redhat.com Cc: Mark Yao mark.yao@rock-chips.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Cc: Thierry Reding thierry.reding@gmail.com Cc: Eric Anholt eric@anholt.net
Jani Nikula (7): drm/edid: use macros for ELD offsets and values drm/edid: set ELD connector type in drm_edid_to_eld() drm/i915: remove redundant ELD connector type update drm/edid: abstract connector ELD clearing drm/edid: build ELD in drm_add_edid_modes() drm/drivers: drop redundant drm_edid_to_eld() calls drm/edid: make drm_edid_to_eld() static
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 1 - drivers/gpu/drm/bridge/analogix-anx78xx.c | 2 - drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 - drivers/gpu/drm/drm_edid.c | 70 +++++++++++++++----------- drivers/gpu/drm/i2c/tda998x_drv.c | 1 - drivers/gpu/drm/i915/intel_dp.c | 1 - drivers/gpu/drm/i915/intel_modes.c | 18 ------- drivers/gpu/drm/mediatek/mtk_hdmi.c | 1 - drivers/gpu/drm/nouveau/nv50_display.c | 5 +- drivers/gpu/drm/radeon/radeon_connectors.c | 1 - drivers/gpu/drm/radeon/radeon_dp_mst.c | 1 - drivers/gpu/drm/rockchip/cdn-dp-core.c | 4 +- drivers/gpu/drm/sti/sti_hdmi.c | 1 - drivers/gpu/drm/tegra/output.c | 1 - drivers/gpu/drm/vc4/vc4_hdmi.c | 1 - include/drm/drm_edid.h | 1 - include/drm/drm_modeset_helper_vtables.h | 3 -- 17 files changed, 44 insertions(+), 70 deletions(-)
-- 2.11.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wed, Nov 01, 2017 at 04:20:56PM +0200, Jani Nikula wrote:
We were recently bitten by drm_edid_to_eld() clearing the connector type, and us failing to set it back for DP. Here's a few ELD related patches to try to unify ELD handling and make it a bit simpler for drivers to get it right.
Apologies for the massive Cc list; it's the maintainers of all drivers that call drm_edid_to_eld().
I'm open to splitting up patch 6 to driver specific patches as needed, but I'd think it would be just fine to merge via drm-misc as-is too.
BR, Jani.
Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Archit Taneja architt@codeaurora.org Cc: Andrzej Hajda a.hajda@samsung.com Cc: Russell King linux@armlinux.org.uk Cc: CK Hu ck.hu@mediatek.com Cc: Philipp Zabel p.zabel@pengutronix.de Cc: Ben Skeggs bskeggs@redhat.com Cc: Mark Yao mark.yao@rock-chips.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Cc: Thierry Reding thierry.reding@gmail.com Cc: Eric Anholt eric@anholt.net
Jani Nikula (7): drm/edid: use macros for ELD offsets and values drm/edid: set ELD connector type in drm_edid_to_eld() drm/i915: remove redundant ELD connector type update drm/edid: abstract connector ELD clearing drm/edid: build ELD in drm_add_edid_modes() drm/drivers: drop redundant drm_edid_to_eld() calls drm/edid: make drm_edid_to_eld() static
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 1 - drivers/gpu/drm/bridge/analogix-anx78xx.c | 2 - drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 - drivers/gpu/drm/drm_edid.c | 70 +++++++++++++++----------- drivers/gpu/drm/i2c/tda998x_drv.c | 1 - drivers/gpu/drm/i915/intel_dp.c | 1 - drivers/gpu/drm/i915/intel_modes.c | 18 ------- drivers/gpu/drm/mediatek/mtk_hdmi.c | 1 - drivers/gpu/drm/nouveau/nv50_display.c | 5 +- drivers/gpu/drm/radeon/radeon_connectors.c | 1 - drivers/gpu/drm/radeon/radeon_dp_mst.c | 1 - drivers/gpu/drm/rockchip/cdn-dp-core.c | 4 +- drivers/gpu/drm/sti/sti_hdmi.c | 1 - drivers/gpu/drm/tegra/output.c | 1 - drivers/gpu/drm/vc4/vc4_hdmi.c | 1 - include/drm/drm_edid.h | 1 - include/drm/drm_modeset_helper_vtables.h | 3 -- 17 files changed, 44 insertions(+), 70 deletions(-)
The series:
Acked-by: Thierry Reding treding@nvidia.com
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Archit Taneja -
Eric Anholt -
Jani Nikula -
Thierry Reding -
Ville Syrjälä