Changelog: v2: - no helper function - A separate patch is made for amdgpu - zte patch is removed because that driver no longer exists
[Why] Copy&paste from Documentation/gpu/todo.rst === Replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi ---------------------------------------------------------------
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Many drivers still call drm_detect_hdmi_monitor() to retrieve the same information, which is less efficient.
Audit each individual driver calling drm_detect_hdmi_monitor() and switch to drm_display_info.is_hdmi if applicable. =====
[How] I did it in two steps: - check that drm_display_info has a correct value. - in that case, replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Almost all occurrences of drm_detect_hdmi_monitor() could be changed. Some small inconsistencies have been solved.
Stats: drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- drivers/gpu/drm/bridge/sii902x.c | 2 +- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- drivers/gpu/drm/drm_edid.c | 11 +++++------ drivers/gpu/drm/exynos/exynos_hdmi.c | 6 ++++-- drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 3 ++- drivers/gpu/drm/gma500/psb_intel_sdvo.c | 6 ++++-- drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +- drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++- drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 4 ++-- drivers/gpu/drm/nouveau/dispnv50/head.c | 8 +------- drivers/gpu/drm/radeon/atombios_encoders.c | 6 +++--- drivers/gpu/drm/radeon/radeon_connectors.c | 15 +++++++++------ drivers/gpu/drm/radeon/radeon_display.c | 2 +- drivers/gpu/drm/radeon/radeon_encoders.c | 4 ++-- drivers/gpu/drm/rockchip/inno_hdmi.c | 4 ++-- drivers/gpu/drm/rockchip/rk3066_hdmi.c | 2 +- drivers/gpu/drm/sti/sti_hdmi.c | 10 ++++++---- drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 4 ++-- drivers/gpu/drm/tegra/hdmi.c | 6 +----- drivers/gpu/drm/vc4/vc4_hdmi.c | 6 +++--- 22 files changed, 55 insertions(+), 55 deletions(-)
Best regards. Claudio Suarez
According to the documentation, drm_add_edid_modes "... Also fills out the &drm_display_info structure and ELD in @connector with any information which can be derived from the edid."
drm_add_edid_modes accepts a struct edid *edid parameter which may have a value or may be null. When it is not null, connector->display_info and connector->eld are updated according to the edid. When edid=NULL, only connector->eld is reset. Reset connector->display_info to be consistent and accurate.
Signed-off-by: Claudio Suarez cssk@net-c.es --- drivers/gpu/drm/drm_edid.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 6325877c5fd6..c643db17782c 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5356,14 +5356,13 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) int num_modes = 0; u32 quirks;
- if (edid == NULL) { - clear_eld(connector); - return 0; - } if (!drm_edid_is_valid(edid)) { + /* edid == NULL or invalid here */ clear_eld(connector); - drm_warn(connector->dev, "%s: EDID invalid.\n", - connector->name); + drm_reset_display_info(connector); + if (edid) + drm_warn(connector->dev, "%s: EDID invalid.\n", + connector->name); return 0; }
On Sat, Oct 16, 2021 at 08:42:14PM +0200, Claudio Suarez wrote:
According to the documentation, drm_add_edid_modes "... Also fills out the &drm_display_info structure and ELD in @connector with any information which can be derived from the edid."
drm_add_edid_modes accepts a struct edid *edid parameter which may have a value or may be null. When it is not null, connector->display_info and connector->eld are updated according to the edid. When edid=NULL, only connector->eld is reset. Reset connector->display_info to be consistent and accurate.
Signed-off-by: Claudio Suarez cssk@net-c.es
drivers/gpu/drm/drm_edid.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 6325877c5fd6..c643db17782c 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5356,14 +5356,13 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) int num_modes = 0; u32 quirks;
- if (edid == NULL) {
clear_eld(connector);return 0;- } if (!drm_edid_is_valid(edid)) {
OK, so drm_edid_is_valid() will happily accept NULL and considers it invalid. You may want to mention that explicitly in the commit message.
/* edid == NULL or invalid here */
drm_warn(connector->dev, "%s: EDID invalid.\n",connector->name);
drm_reset_display_info(connector);if (edid)drm_warn(connector->dev, "%s: EDID invalid.\n",connector->name);
Could you respin this to use the standard [CONNECTOR:%d:%s] form while at it? Or I guess a patch to mass convert the whole drm_edid.c might be another option.
Patch looks good. Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
return 0;}
-- 2.33.0
On Tue, Oct 19, 2021 at 09:35:08PM +0300, Ville Syrjälä wrote:
On Sat, Oct 16, 2021 at 08:42:14PM +0200, Claudio Suarez wrote:
According to the documentation, drm_add_edid_modes "... Also fills out the &drm_display_info structure and ELD in @connector with any information which can be derived from the edid."
drm_add_edid_modes accepts a struct edid *edid parameter which may have a value or may be null. When it is not null, connector->display_info and connector->eld are updated according to the edid. When edid=NULL, only connector->eld is reset. Reset connector->display_info to be consistent and accurate.
Signed-off-by: Claudio Suarez cssk@net-c.es
drivers/gpu/drm/drm_edid.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 6325877c5fd6..c643db17782c 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5356,14 +5356,13 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) int num_modes = 0; u32 quirks;
- if (edid == NULL) {
clear_eld(connector);return 0;- } if (!drm_edid_is_valid(edid)) {
OK, so drm_edid_is_valid() will happily accept NULL and considers it invalid. You may want to mention that explicitly in the commit message.
Thank you for your comments, I appreciate :) I'm sending new mails with the new commit messages.
/* edid == NULL or invalid here */
drm_warn(connector->dev, "%s: EDID invalid.\n",connector->name);
drm_reset_display_info(connector);if (edid)drm_warn(connector->dev, "%s: EDID invalid.\n",connector->name);Could you respin this to use the standard [CONNECTOR:%d:%s] form while at it? Or I guess a patch to mass convert the whole drm_edid.c might be another option.
Good point. I like the idea of a new patch. I'll start working on it. I can change this drm_warn here to avoid merge conflicts.
Patch looks good. Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
Thanks!
BR Claudio Suarez.
According to the documentation, drm_add_edid_modes "... Also fills out the &drm_display_info structure and ELD in @connector with any information which can be derived from the edid."
drm_add_edid_modes accepts a struct edid *edid parameter which may have a value or may be null. When it is not null, connector->display_info and connector->eld are updated according to the edid. When edid=NULL, only connector->eld is reset. Reset connector->display_info to be consistent and accurate.
Since drm_edid_is_valid() considers NULL as an invalid EDID, simplify the code to avoid duplicating code in the case of NULL/error.
Signed-off-by: Claudio Suarez cssk@net-c.es --- drivers/gpu/drm/drm_edid.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 6325877c5fd6..a019a26ede7a 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5356,14 +5356,14 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) int num_modes = 0; u32 quirks;
- if (edid == NULL) { - clear_eld(connector); - return 0; - } if (!drm_edid_is_valid(edid)) { + /* edid == NULL or invalid here */ clear_eld(connector); - drm_warn(connector->dev, "%s: EDID invalid.\n", - connector->name); + drm_reset_display_info(connector); + if (edid) + drm_warn(connector->dev, + "[CONNECTOR:%d:%s] EDID invalid.\n", + connector->base.id, connector->name); return 0; }
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Use this value instead of calling drm_detect_hdmi_monitor() to avoid a second parse.
This is a TODO task in Documentation/gpu/todo.rst
Signed-off-by: Claudio Suarez cssk@net-c.es --- drivers/gpu/drm/vc4/vc4_hdmi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index b4b4653fe301..d531e4c501eb 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -182,7 +182,8 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
if (edid) { cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, edid); - vc4_hdmi->encoder.hdmi_monitor = drm_detect_hdmi_monitor(edid); + vc4_hdmi->encoder.hdmi_monitor = + connector->display_info.is_hdmi; kfree(edid); } } @@ -212,10 +213,9 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector) if (!edid) return -ENODEV;
- vc4_encoder->hdmi_monitor = drm_detect_hdmi_monitor(edid); - drm_connector_update_edid_property(connector, edid); ret = drm_add_edid_modes(connector, edid); + vc4_encoder->hdmi_monitor = connector->display_info.is_hdmi; kfree(edid);
if (vc4_hdmi->disable_4kp60) {
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information is less efficient. Change to drm_display_info.is_hdmi
This is a TODO task in Documentation/gpu/todo.rst
Also, correct an inacurracy or bug in radeon_connector_get_edid()/radeon_connector_free_edid(). Two variables have EDID data: - struct radeon_connector.edid - struct drm_connector.edid_blob_ptr The second is updated by calling drm_connector_update_edid_property() or drm_get_edid() - which internally calls drm_connector_update_edid_property(). drm_display_info.is_hdmi is updated when this function is called. radeon_connector_get_edid() calls drm_get_edid() to update drm_connector.edid_blob_ptr/drm_display_info only in some cases. Change it to be always up to date, so drm_display_info is always correct. As counterpart, reset these values in radeon_connector_free_edid().
This second change is necessary for the previous one to work properly.
Signed-off-by: Claudio Suarez cssk@net-c.es --- drivers/gpu/drm/radeon/atombios_encoders.c | 6 +++--- drivers/gpu/drm/radeon/radeon_connectors.c | 15 +++++++++------ drivers/gpu/drm/radeon/radeon_display.c | 2 +- drivers/gpu/drm/radeon/radeon_encoders.c | 4 ++-- 4 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c index 0fce73b9a646..844f61a36f20 100644 --- a/drivers/gpu/drm/radeon/atombios_encoders.c +++ b/drivers/gpu/drm/radeon/atombios_encoders.c @@ -713,7 +713,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder) if (radeon_connector->use_digital && (radeon_connector->audio == RADEON_AUDIO_ENABLE)) return ATOM_ENCODER_MODE_HDMI; - else if (drm_detect_hdmi_monitor(radeon_connector_edid(connector)) && + else if (connector->display_info.is_hdmi && (radeon_connector->audio == RADEON_AUDIO_AUTO)) return ATOM_ENCODER_MODE_HDMI; else if (radeon_connector->use_digital) @@ -732,7 +732,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder) if (radeon_audio != 0) { if (radeon_connector->audio == RADEON_AUDIO_ENABLE) return ATOM_ENCODER_MODE_HDMI; - else if (drm_detect_hdmi_monitor(radeon_connector_edid(connector)) && + else if (connector->display_info.is_hdmi && (radeon_connector->audio == RADEON_AUDIO_AUTO)) return ATOM_ENCODER_MODE_HDMI; else @@ -756,7 +756,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder) } else if (radeon_audio != 0) { if (radeon_connector->audio == RADEON_AUDIO_ENABLE) return ATOM_ENCODER_MODE_HDMI; - else if (drm_detect_hdmi_monitor(radeon_connector_edid(connector)) && + else if (connector->display_info.is_hdmi && (radeon_connector->audio == RADEON_AUDIO_AUTO)) return ATOM_ENCODER_MODE_HDMI; else diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c index 607ad5620bd9..deaae181ac59 100644 --- a/drivers/gpu/drm/radeon/radeon_connectors.c +++ b/drivers/gpu/drm/radeon/radeon_connectors.c @@ -130,7 +130,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector) case DRM_MODE_CONNECTOR_DVII: case DRM_MODE_CONNECTOR_HDMIB: if (radeon_connector->use_digital) { - if (drm_detect_hdmi_monitor(radeon_connector_edid(connector))) { + if (connector->display_info.is_hdmi) { if (connector->display_info.bpc) bpc = connector->display_info.bpc; } @@ -138,7 +138,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector) break; case DRM_MODE_CONNECTOR_DVID: case DRM_MODE_CONNECTOR_HDMIA: - if (drm_detect_hdmi_monitor(radeon_connector_edid(connector))) { + if (connector->display_info.is_hdmi) { if (connector->display_info.bpc) bpc = connector->display_info.bpc; } @@ -147,7 +147,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector) dig_connector = radeon_connector->con_priv; if ((dig_connector->dp_sink_type == CONNECTOR_OBJECT_ID_DISPLAYPORT) || (dig_connector->dp_sink_type == CONNECTOR_OBJECT_ID_eDP) || - drm_detect_hdmi_monitor(radeon_connector_edid(connector))) { + connector->display_info.is_hdmi) { if (connector->display_info.bpc) bpc = connector->display_info.bpc; } @@ -171,7 +171,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector) break; }
- if (drm_detect_hdmi_monitor(radeon_connector_edid(connector))) { + if (connector->display_info.is_hdmi) { /* hdmi deep color only implemented on DCE4+ */ if ((bpc > 8) && !ASIC_IS_DCE4(rdev)) { DRM_DEBUG("%s: HDMI deep color %d bpc unsupported. Using 8 bpc.\n", @@ -348,6 +348,8 @@ static void radeon_connector_get_edid(struct drm_connector *connector) /* some servers provide a hardcoded edid in rom for KVMs */ radeon_connector->edid = radeon_bios_get_hardcoded_edid(rdev); } + if (radeon_connector->edid) + drm_connector_update_edid_property(connector, radeon_connector->edid); } }
@@ -358,6 +360,7 @@ static void radeon_connector_free_edid(struct drm_connector *connector) if (radeon_connector->edid) { kfree(radeon_connector->edid); radeon_connector->edid = NULL; + drm_connector_update_edid_property(connector, NULL); } }
@@ -1496,7 +1499,7 @@ static enum drm_mode_status radeon_dvi_mode_valid(struct drm_connector *connecto (radeon_connector->connector_object_id == CONNECTOR_OBJECT_ID_DUAL_LINK_DVI_D) || (radeon_connector->connector_object_id == CONNECTOR_OBJECT_ID_HDMI_TYPE_B)) return MODE_OK; - else if (ASIC_IS_DCE6(rdev) && drm_detect_hdmi_monitor(radeon_connector_edid(connector))) { + else if (ASIC_IS_DCE6(rdev) && connector->display_info.is_hdmi) { /* HDMI 1.3+ supports max clock of 340 Mhz */ if (mode->clock > 340000) return MODE_CLOCK_HIGH; @@ -1804,7 +1807,7 @@ static enum drm_mode_status radeon_dp_mode_valid(struct drm_connector *connector (radeon_dig_connector->dp_sink_type == CONNECTOR_OBJECT_ID_eDP)) { return radeon_dp_mode_valid_helper(connector, mode); } else { - if (ASIC_IS_DCE6(rdev) && drm_detect_hdmi_monitor(radeon_connector_edid(connector))) { + if (ASIC_IS_DCE6(rdev) && connector->display_info.is_hdmi) { /* HDMI 1.3+ supports max clock of 340 Mhz */ if (mode->clock > 340000) return MODE_CLOCK_HIGH; diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index 573154268d43..f406933a8f1d 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -1720,7 +1720,7 @@ bool radeon_crtc_scaling_mode_fixup(struct drm_crtc *crtc, (!(mode->flags & DRM_MODE_FLAG_INTERLACE)) && ((radeon_encoder->underscan_type == UNDERSCAN_ON) || ((radeon_encoder->underscan_type == UNDERSCAN_AUTO) && - drm_detect_hdmi_monitor(radeon_connector_edid(connector)) && + connector->display_info.is_hdmi && is_hdtv_mode(mode)))) { if (radeon_encoder->underscan_hborder != 0) radeon_crtc->h_border = radeon_encoder->underscan_hborder; diff --git a/drivers/gpu/drm/radeon/radeon_encoders.c b/drivers/gpu/drm/radeon/radeon_encoders.c index 46549d5179ee..b999464f213a 100644 --- a/drivers/gpu/drm/radeon/radeon_encoders.c +++ b/drivers/gpu/drm/radeon/radeon_encoders.c @@ -383,7 +383,7 @@ bool radeon_dig_monitor_is_duallink(struct drm_encoder *encoder, case DRM_MODE_CONNECTOR_HDMIB: if (radeon_connector->use_digital) { /* HDMI 1.3 supports up to 340 Mhz over single link */ - if (ASIC_IS_DCE6(rdev) && drm_detect_hdmi_monitor(radeon_connector_edid(connector))) { + if (ASIC_IS_DCE6(rdev) && connector->display_info.is_hdmi) { if (pixel_clock > 340000) return true; else @@ -408,7 +408,7 @@ bool radeon_dig_monitor_is_duallink(struct drm_encoder *encoder, return false; else { /* HDMI 1.3 supports up to 340 Mhz over single link */ - if (ASIC_IS_DCE6(rdev) && drm_detect_hdmi_monitor(radeon_connector_edid(connector))) { + if (ASIC_IS_DCE6(rdev) && connector->display_info.is_hdmi) { if (pixel_clock > 340000) return true; else
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez cssk@net-c.es --- drivers/gpu/drm/tegra/hdmi.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c index e5d2a4026028..21571221b49b 100644 --- a/drivers/gpu/drm/tegra/hdmi.c +++ b/drivers/gpu/drm/tegra/hdmi.c @@ -831,14 +831,10 @@ static void tegra_hdmi_setup_tmds(struct tegra_hdmi *hdmi,
static bool tegra_output_is_hdmi(struct tegra_output *output) { - struct edid *edid; - if (!output->connector.edid_blob_ptr) return false;
- edid = (struct edid *)output->connector.edid_blob_ptr->data; - - return drm_detect_hdmi_monitor(edid); + return output->connector.display_info.is_hdmi; }
static enum drm_connector_status
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez cssk@net-c.es --- drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 3 ++- drivers/gpu/drm/gma500/psb_intel_sdvo.c | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c index e525689f84f0..d9db5d52d52e 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c +++ b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c @@ -130,6 +130,7 @@ static enum drm_connector_status cdv_hdmi_detect( struct edid *edid = NULL; enum drm_connector_status status = connector_status_disconnected;
+ /* This updates connector->display_info */ edid = drm_get_edid(connector, &gma_encoder->i2c_bus->adapter);
hdmi_priv->has_hdmi_sink = false; @@ -138,7 +139,7 @@ static enum drm_connector_status cdv_hdmi_detect( if (edid->input & DRM_EDID_INPUT_DIGITAL) { status = connector_status_connected; hdmi_priv->has_hdmi_sink = - drm_detect_hdmi_monitor(edid); + connector->display_info.is_hdmi; hdmi_priv->has_hdmi_audio = drm_detect_monitor_audio(edid); } diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c b/drivers/gpu/drm/gma500/psb_intel_sdvo.c index 355da2856389..5ef49d17de98 100644 --- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c +++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c @@ -1266,8 +1266,10 @@ psb_intel_sdvo_hdmi_sink_detect(struct drm_connector *connector) if (edid->input & DRM_EDID_INPUT_DIGITAL) { status = connector_status_connected; if (psb_intel_sdvo->is_hdmi) { - psb_intel_sdvo->has_hdmi_monitor = drm_detect_hdmi_monitor(edid); - psb_intel_sdvo->has_hdmi_audio = drm_detect_monitor_audio(edid); + psb_intel_sdvo->has_hdmi_monitor = + connector->display_info.is_hdmi; + psb_intel_sdvo->has_hdmi_audio = + drm_detect_monitor_audio(edid); } } else status = connector_status_disconnected;
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez cssk@net-c.es --- drivers/gpu/drm/exynos/exynos_hdmi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 7655142a4651..a563d6386abe 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -893,12 +893,14 @@ static int hdmi_get_modes(struct drm_connector *connector) if (!edid) return -ENODEV;
- hdata->dvi_mode = !drm_detect_hdmi_monitor(edid); + /* This updates connector->display_info */ + drm_connector_update_edid_property(connector, edid); + + hdata->dvi_mode = !connector->display_info.is_hdmi; DRM_DEV_DEBUG_KMS(hdata->dev, "%s : width[%d] x height[%d]\n", (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"), edid->width_cm, edid->height_cm);
- drm_connector_update_edid_property(connector, edid); cec_notifier_set_phys_addr_from_edid(hdata->notifier, edid);
ret = drm_add_edid_modes(connector, edid);
Hi,
21. 10. 17. 오전 3:42에 Claudio Suarez 이(가) 쓴 글:
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez cssk@net-c.es
drivers/gpu/drm/exynos/exynos_hdmi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 7655142a4651..a563d6386abe 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -893,12 +893,14 @@ static int hdmi_get_modes(struct drm_connector *connector) if (!edid) return -ENODEV;
- hdata->dvi_mode = !drm_detect_hdmi_monitor(edid);
- /* This updates connector->display_info */
- drm_connector_update_edid_property(connector, edid);
- hdata->dvi_mode = !connector->display_info.is_hdmi;
Thanks for correcting this. Yeah, we should use drm_display_info.is_hdmi parsed from EDID. https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/driver...
Signed-off-by: Inki Dae inki.dae@samsung.com
Thanks, Inki Dae
DRM_DEV_DEBUG_KMS(hdata->dev, "%s : width[%d] x height[%d]\n", (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"), edid->width_cm, edid->height_cm);
drm_connector_update_edid_property(connector, edid); cec_notifier_set_phys_addr_from_edid(hdata->notifier, edid);
ret = drm_add_edid_modes(connector, edid);
21. 10. 27. 오전 7:28에 Inki Dae 이(가) 쓴 글:
Hi,
- 오전 3:42에 Claudio Suarez 이(가) 쓴 글:
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez cssk@net-c.es
drivers/gpu/drm/exynos/exynos_hdmi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 7655142a4651..a563d6386abe 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -893,12 +893,14 @@ static int hdmi_get_modes(struct drm_connector *connector) if (!edid) return -ENODEV;
- hdata->dvi_mode = !drm_detect_hdmi_monitor(edid);
- /* This updates connector->display_info */
- drm_connector_update_edid_property(connector, edid);
- hdata->dvi_mode = !connector->display_info.is_hdmi;
Thanks for correcting this. Yeah, we should use drm_display_info.is_hdmi parsed from EDID. https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/driver...
Signed-off-by: Inki Dae inki.dae@samsung.com
My mistake. Please, ignore above Signed-off-by.
Acked-by : Inki Dae inki.dae@samsung.com instead.
Thanks, Inki Dae
Thanks, Inki Dae
DRM_DEV_DEBUG_KMS(hdata->dev, "%s : width[%d] x height[%d]\n", (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"), edid->width_cm, edid->height_cm);
drm_connector_update_edid_property(connector, edid); cec_notifier_set_phys_addr_from_edid(hdata->notifier, edid);
ret = drm_add_edid_modes(connector, edid);
On Wed, Oct 27, 2021 at 07:28:45AM +0900, Inki Dae wrote:
Hi,
- 오전 3:42에 Claudio Suarez 이(가) 쓴 글:
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez cssk@net-c.es
drivers/gpu/drm/exynos/exynos_hdmi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 7655142a4651..a563d6386abe 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -893,12 +893,14 @@ static int hdmi_get_modes(struct drm_connector *connector) if (!edid) return -ENODEV;
- hdata->dvi_mode = !drm_detect_hdmi_monitor(edid);
- /* This updates connector->display_info */
- drm_connector_update_edid_property(connector, edid);
- hdata->dvi_mode = !connector->display_info.is_hdmi;
Thanks for correcting this. Yeah, we should use drm_display_info.is_hdmi parsed from EDID. https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/driver...
Signed-off-by: Inki Dae inki.dae@samsung.com
Thank you, Inki.
Best regards Claudio Suarez
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez cssk@net-c.es --- drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c index 58707a1f3878..07585092f919 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c @@ -364,8 +364,8 @@ static int msm_hdmi_connector_get_modes(struct drm_connector *connector)
hdmi_write(hdmi, REG_HDMI_CTRL, hdmi_ctrl);
- hdmi->hdmi_mode = drm_detect_hdmi_monitor(edid); drm_connector_update_edid_property(connector, edid); + hdmi->hdmi_mode = connector->display_info.is_hdmi;
if (edid) { ret = drm_add_edid_modes(connector, edid);
On 16/10/2021 21:42, Claudio Suarez wrote:
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez cssk@net-c.es
Reviewed-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org
drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c index 58707a1f3878..07585092f919 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c @@ -364,8 +364,8 @@ static int msm_hdmi_connector_get_modes(struct drm_connector *connector)
hdmi_write(hdmi, REG_HDMI_CTRL, hdmi_ctrl);
- hdmi->hdmi_mode = drm_detect_hdmi_monitor(edid); drm_connector_update_edid_property(connector, edid);
hdmi->hdmi_mode = connector->display_info.is_hdmi;
if (edid) { ret = drm_add_edid_modes(connector, edid);
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez cssk@net-c.es --- drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c index 2f2c9f0a1071..f57bedbbeeb8 100644 --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c @@ -215,11 +215,11 @@ static int sun4i_hdmi_get_modes(struct drm_connector *connector) if (!edid) return 0;
- hdmi->hdmi_monitor = drm_detect_hdmi_monitor(edid); + drm_connector_update_edid_property(connector, edid); + hdmi->hdmi_monitor = connector->display_info.is_hdmi; DRM_DEBUG_DRIVER("Monitor is %s monitor\n", hdmi->hdmi_monitor ? "an HDMI" : "a DVI");
- drm_connector_update_edid_property(connector, edid); cec_s_phys_addr_from_edid(hdmi->cec_adap, edid); ret = drm_add_edid_modes(connector, edid); kfree(edid);
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez cssk@net-c.es --- drivers/gpu/drm/sti/sti_hdmi.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c index f3ace11209dd..3f8b04a1407e 100644 --- a/drivers/gpu/drm/sti/sti_hdmi.c +++ b/drivers/gpu/drm/sti/sti_hdmi.c @@ -984,14 +984,16 @@ static int sti_hdmi_connector_get_modes(struct drm_connector *connector) if (!edid) goto fail;
- hdmi->hdmi_monitor = drm_detect_hdmi_monitor(edid); - DRM_DEBUG_KMS("%s : %dx%d cm\n", - (hdmi->hdmi_monitor ? "hdmi monitor" : "dvi monitor"), - edid->width_cm, edid->height_cm); cec_notifier_set_phys_addr_from_edid(hdmi->notifier, edid);
count = drm_add_edid_modes(connector, edid); + + /* This updates connector->display_info */ drm_connector_update_edid_property(connector, edid); + hdmi->hdmi_monitor = connector->display_info.is_hdmi; + DRM_DEBUG_KMS("%s : %dx%d cm\n", + (hdmi->hdmi_monitor ? "hdmi monitor" : "dvi monitor"), + edid->width_cm, edid->height_cm);
kfree(edid); return count;
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez cssk@net-c.es --- drivers/gpu/drm/rockchip/inno_hdmi.c | 4 ++-- drivers/gpu/drm/rockchip/rk3066_hdmi.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c index 7afdc54eb3ec..d479f230833e 100644 --- a/drivers/gpu/drm/rockchip/inno_hdmi.c +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c @@ -553,9 +553,9 @@ static int inno_hdmi_connector_get_modes(struct drm_connector *connector)
edid = drm_get_edid(connector, hdmi->ddc); if (edid) { - hdmi->hdmi_data.sink_is_hdmi = drm_detect_hdmi_monitor(edid); - hdmi->hdmi_data.sink_has_audio = drm_detect_monitor_audio(edid); drm_connector_update_edid_property(connector, edid); + hdmi->hdmi_data.sink_is_hdmi = connector->display_info.is_hdmi; + hdmi->hdmi_data.sink_has_audio = drm_detect_monitor_audio(edid); ret = drm_add_edid_modes(connector, edid); kfree(edid); } diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c b/drivers/gpu/drm/rockchip/rk3066_hdmi.c index 1c546c3a8998..03aaae39cf61 100644 --- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c +++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c @@ -472,8 +472,8 @@ static int rk3066_hdmi_connector_get_modes(struct drm_connector *connector)
edid = drm_get_edid(connector, hdmi->ddc); if (edid) { - hdmi->hdmi_data.sink_is_hdmi = drm_detect_hdmi_monitor(edid); drm_connector_update_edid_property(connector, edid); + hdmi->hdmi_data.sink_is_hdmi = connector->display_info.is_hdmi; ret = drm_add_edid_modes(connector, edid); kfree(edid); }
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi where possible
Signed-off-by: Claudio Suarez cssk@net-c.es --- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- drivers/gpu/drm/bridge/sii902x.c | 2 +- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 76555ae64e9c..f6891280a58d 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -617,7 +617,7 @@ static struct edid *adv7511_get_edid(struct adv7511 *adv7511, __adv7511_power_off(adv7511);
adv7511_set_config_csc(adv7511, connector, adv7511->rgb, - drm_detect_hdmi_monitor(edid)); + connector->display_info.is_hdmi);
cec_s_phys_addr_from_edid(adv7511->cec_adap, edid);
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index 89558e581530..5719be0a03c7 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -283,7 +283,7 @@ static int sii902x_get_modes(struct drm_connector *connector) edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]); drm_connector_update_edid_property(connector, edid); if (edid) { - if (drm_detect_hdmi_monitor(edid)) + if (connector->display_info.is_hdmi) output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
num = drm_add_edid_modes(connector, edid); diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index f08d0fded61f..33f0afb6b646 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -2359,7 +2359,7 @@ static struct edid *dw_hdmi_get_edid(struct dw_hdmi *hdmi, dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n", edid->width_cm, edid->height_cm);
- hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid); + hdmi->sink_is_hdmi = connector->display_info.is_hdmi; hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
return edid;
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez cssk@net-c.es --- drivers/gpu/drm/nouveau/dispnv50/disp.c | 4 ++-- drivers/gpu/drm/nouveau/dispnv50/head.c | 8 +------- 2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index d7b9f7f8c9e3..b3c7199aa63d 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -844,7 +844,7 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct nouveau_crtc *nv_crtc, int ret; int size;
- if (!drm_detect_hdmi_monitor(nv_connector->edid)) + if (!nv_connector->base.display_info.is_hdmi) return;
hdmi = &nv_connector->base.display_info.hdmi; @@ -1745,7 +1745,7 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *sta */ if (mode->clock >= 165000 && nv_encoder->dcb->duallink_possible && - !drm_detect_hdmi_monitor(nv_connector->edid)) + !nv_connector->base.display_info.is_hdmi) proto = NV507D_SOR_SET_CONTROL_PROTOCOL_DUAL_TMDS; } else { proto = NV507D_SOR_SET_CONTROL_PROTOCOL_SINGLE_TMDS_B; diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c b/drivers/gpu/drm/nouveau/dispnv50/head.c index d66f97280282..29d6c010ac13 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/head.c +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c @@ -127,14 +127,8 @@ nv50_head_atomic_check_view(struct nv50_head_atom *armh, struct drm_display_mode *omode = &asyh->state.adjusted_mode; struct drm_display_mode *umode = &asyh->state.mode; int mode = asyc->scaler.mode; - struct edid *edid; int umode_vdisplay, omode_hdisplay, omode_vdisplay;
- if (connector->edid_blob_ptr) - edid = (struct edid *)connector->edid_blob_ptr->data; - else - edid = NULL; - if (!asyc->scaler.full) { if (mode == DRM_MODE_SCALE_NONE) omode = umode; @@ -162,7 +156,7 @@ nv50_head_atomic_check_view(struct nv50_head_atom *armh, */ if ((asyc->scaler.underscan.mode == UNDERSCAN_ON || (asyc->scaler.underscan.mode == UNDERSCAN_AUTO && - drm_detect_hdmi_monitor(edid)))) { + connector->display_info.is_hdmi))) { u32 bX = asyc->scaler.underscan.hborder; u32 bY = asyc->scaler.underscan.vborder; u32 r = (asyh->view.oH << 19) / asyh->view.oW;
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi where possible.
This is a TODO task in Documentation/gpu/todo.rst
Signed-off-by: Claudio Suarez cssk@net-c.es --- drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +- drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index b04685bb6439..008e5b0ba408 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2355,7 +2355,7 @@ intel_hdmi_set_edid(struct drm_connector *connector) to_intel_connector(connector)->detect_edid = edid; if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) { intel_hdmi->has_audio = drm_detect_monitor_audio(edid); - intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid); + intel_hdmi->has_hdmi_sink = connector->display_info.is_hdmi;
connected = true; } diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c index 6cb27599ea03..b4065e4df644 100644 --- a/drivers/gpu/drm/i915/display/intel_sdvo.c +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c @@ -2060,8 +2060,9 @@ intel_sdvo_tmds_sink_detect(struct drm_connector *connector) if (edid->input & DRM_EDID_INPUT_DIGITAL) { status = connector_status_connected; if (intel_sdvo_connector->is_hdmi) { - intel_sdvo->has_hdmi_monitor = drm_detect_hdmi_monitor(edid); intel_sdvo->has_hdmi_audio = drm_detect_monitor_audio(edid); + intel_sdvo->has_hdmi_monitor = + connector->display_info.is_hdmi; } } else status = connector_status_disconnected;
On Sat, Oct 16, 2021 at 08:42:26PM +0200, Claudio Suarez wrote:
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi where possible.
We still need proof in the commit message that display_info is actually populated by the time this gets called.
This is a TODO task in Documentation/gpu/todo.rst
Signed-off-by: Claudio Suarez cssk@net-c.es
drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +- drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index b04685bb6439..008e5b0ba408 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2355,7 +2355,7 @@ intel_hdmi_set_edid(struct drm_connector *connector) to_intel_connector(connector)->detect_edid = edid; if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) { intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
intel_hdmi->has_hdmi_sink = connector->display_info.is_hdmi;connected = true; }
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c index 6cb27599ea03..b4065e4df644 100644 --- a/drivers/gpu/drm/i915/display/intel_sdvo.c +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c @@ -2060,8 +2060,9 @@ intel_sdvo_tmds_sink_detect(struct drm_connector *connector) if (edid->input & DRM_EDID_INPUT_DIGITAL) { status = connector_status_connected; if (intel_sdvo_connector->is_hdmi) {
intel_sdvo->has_hdmi_monitor = drm_detect_hdmi_monitor(edid); intel_sdvo->has_hdmi_audio = drm_detect_monitor_audio(edid);
intel_sdvo->has_hdmi_monitor =connector->display_info.is_hdmi; }-- 2.33.0
drm_get_edid() internally calls to drm_connector_update_edid_property() and then drm_add_display_info(), which parses the EDID. This happens in the function intel_hdmi_set_edid() and intel_sdvo_tmds_sink_detect() (via intel_sdvo_get_edid()).
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi
This is a TODO task in Documentation/gpu/todo.rst
Signed-off-by: Claudio Suarez cssk@net-c.es --- drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +- drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index b04685bb6439..008e5b0ba408 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2355,7 +2355,7 @@ intel_hdmi_set_edid(struct drm_connector *connector) to_intel_connector(connector)->detect_edid = edid; if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) { intel_hdmi->has_audio = drm_detect_monitor_audio(edid); - intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid); + intel_hdmi->has_hdmi_sink = connector->display_info.is_hdmi;
connected = true; } diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c index 6cb27599ea03..b4065e4df644 100644 --- a/drivers/gpu/drm/i915/display/intel_sdvo.c +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c @@ -2060,8 +2060,9 @@ intel_sdvo_tmds_sink_detect(struct drm_connector *connector) if (edid->input & DRM_EDID_INPUT_DIGITAL) { status = connector_status_connected; if (intel_sdvo_connector->is_hdmi) { - intel_sdvo->has_hdmi_monitor = drm_detect_hdmi_monitor(edid); intel_sdvo->has_hdmi_audio = drm_detect_monitor_audio(edid); + intel_sdvo->has_hdmi_monitor = + connector->display_info.is_hdmi; } } else status = connector_status_disconnected;
On Wed, Oct 20, 2021 at 12:51:21AM +0200, Claudio Suarez wrote:
drm_get_edid() internally calls to drm_connector_update_edid_property() and then drm_add_display_info(), which parses the EDID. This happens in the function intel_hdmi_set_edid() and intel_sdvo_tmds_sink_detect() (via intel_sdvo_get_edid()).
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi
I meant we need to examine all call chains that can lead to .detect() to make sure all of them do in fact update the display_info beforehand.
This is a TODO task in Documentation/gpu/todo.rst
Signed-off-by: Claudio Suarez cssk@net-c.es
drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +- drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index b04685bb6439..008e5b0ba408 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2355,7 +2355,7 @@ intel_hdmi_set_edid(struct drm_connector *connector) to_intel_connector(connector)->detect_edid = edid; if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) { intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
intel_hdmi->has_hdmi_sink = connector->display_info.is_hdmi;connected = true; }
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c index 6cb27599ea03..b4065e4df644 100644 --- a/drivers/gpu/drm/i915/display/intel_sdvo.c +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c @@ -2060,8 +2060,9 @@ intel_sdvo_tmds_sink_detect(struct drm_connector *connector) if (edid->input & DRM_EDID_INPUT_DIGITAL) { status = connector_status_connected; if (intel_sdvo_connector->is_hdmi) {
intel_sdvo->has_hdmi_monitor = drm_detect_hdmi_monitor(edid); intel_sdvo->has_hdmi_audio = drm_detect_monitor_audio(edid);
intel_sdvo->has_hdmi_monitor =connector->display_info.is_hdmi; }-- 2.33.0
On Thu, Oct 21, 2021 at 04:49:59PM +0300, Ville Syrjälä wrote:
On Wed, Oct 20, 2021 at 12:51:21AM +0200, Claudio Suarez wrote:
drm_get_edid() internally calls to drm_connector_update_edid_property() and then drm_add_display_info(), which parses the EDID. This happens in the function intel_hdmi_set_edid() and intel_sdvo_tmds_sink_detect() (via intel_sdvo_get_edid()).
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi
I meant we need to examine all call chains that can lead to .detect() to make sure all of them do in fact update the display_info beforehand.
Well, I studied it carefully and, yes, all call chains that can lead to drm_display_info.is_hdmi / drm_detect_hdmi_monitor() update display_info beforehand. In the case that this doesn't happen, the code is unchanged.
Do you want I explain the changes in the code here again ? Or do you want to me change the commit message to be more clear ? In the first case, I can write here a detailed explanation. In the second case I can make a longer commit message.
Or both?
Best Regards, Claudio Suarez.
On Fri, Oct 22, 2021 at 12:25:33PM +0200, Claudio Suarez wrote:
On Thu, Oct 21, 2021 at 04:49:59PM +0300, Ville Syrjälä wrote:
On Wed, Oct 20, 2021 at 12:51:21AM +0200, Claudio Suarez wrote:
drm_get_edid() internally calls to drm_connector_update_edid_property() and then drm_add_display_info(), which parses the EDID. This happens in the function intel_hdmi_set_edid() and intel_sdvo_tmds_sink_detect() (via intel_sdvo_get_edid()).
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi
I meant we need to examine all call chains that can lead to .detect() to make sure all of them do in fact update the display_info beforehand.
Well, I studied it carefully and, yes, all call chains that can lead to drm_display_info.is_hdmi / drm_detect_hdmi_monitor() update display_info beforehand. In the case that this doesn't happen, the code is unchanged.
Do you want I explain the changes in the code here again ? Or do you want to me change the commit message to be more clear ? In the first case, I can write here a detailed explanation. In the second case I can make a longer commit message.
Or both?
I want all those call chains explained in the commit message, otherwise I have no easy way to confirm whether the change is correct or not.
On Fri, Oct 22, 2021 at 03:01:52PM +0300, Ville Syrjälä wrote:
On Fri, Oct 22, 2021 at 12:25:33PM +0200, Claudio Suarez wrote:
On Thu, Oct 21, 2021 at 04:49:59PM +0300, Ville Syrjälä wrote:
On Wed, Oct 20, 2021 at 12:51:21AM +0200, Claudio Suarez wrote:
drm_get_edid() internally calls to drm_connector_update_edid_property() and then drm_add_display_info(), which parses the EDID. This happens in the function intel_hdmi_set_edid() and intel_sdvo_tmds_sink_detect() (via intel_sdvo_get_edid()).
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi
I meant we need to examine all call chains that can lead to .detect() to make sure all of them do in fact update the display_info beforehand.
Well, I studied it carefully and, yes, all call chains that can lead to drm_display_info.is_hdmi / drm_detect_hdmi_monitor() update display_info beforehand. In the case that this doesn't happen, the code is unchanged.
Do you want I explain the changes in the code here again ? Or do you want to me change the commit message to be more clear ? In the first case, I can write here a detailed explanation. In the second case I can make a longer commit message.
Or both?
I want all those call chains explained in the commit message, otherwise I have no easy way to confirm whether the change is correct or not.
Hmm. OK, so I had a bit of a dig around and seems that what we do now .detect()->drm_get_edid()->drm_connector_update_edid_property()->drm_add_display_info()
Now the question is when did that start happening? Looks like it was commit 4b4df570b41d ("drm: Update edid-derived drm_display_info fields at edid property set [v2]") that started to call drm_add_display_info() from drm_connector_update_edid_property(), and then commit 5186421cbfe2 ("drm: Introduce epoch counter to drm_connector") started to call drm_connector_update_edid_property() from drm_get_edid(). Before both of those commits were in place display_info would still contain some stale garbage during .detect().
That is the story I think we want in these commit messages since it a) explains why the old code was directly parsing the edid instead b) why it's now safe to change this
PS. connector->force handling in drm_get_edid() looks a bit busted since it doesn't call drm_connector_update_edid_property() at all in some cases. I think there might be some path that leads there anywya if/when we change connector->force, but we should fix drm_get_edid() to do the right thing regarless.
On Fri, Oct 22, 2021 at 03:22:57PM +0300, Ville Syrjälä wrote:
On Fri, Oct 22, 2021 at 03:01:52PM +0300, Ville Syrjälä wrote:
On Fri, Oct 22, 2021 at 12:25:33PM +0200, Claudio Suarez wrote:
On Thu, Oct 21, 2021 at 04:49:59PM +0300, Ville Syrjälä wrote:
On Wed, Oct 20, 2021 at 12:51:21AM +0200, Claudio Suarez wrote:
drm_get_edid() internally calls to drm_connector_update_edid_property() and then drm_add_display_info(), which parses the EDID. This happens in the function intel_hdmi_set_edid() and intel_sdvo_tmds_sink_detect() (via intel_sdvo_get_edid()).
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi
I meant we need to examine all call chains that can lead to .detect() to make sure all of them do in fact update the display_info beforehand.
Well, I studied it carefully and, yes, all call chains that can lead to drm_display_info.is_hdmi / drm_detect_hdmi_monitor() update display_info beforehand. In the case that this doesn't happen, the code is unchanged.
Do you want I explain the changes in the code here again ? Or do you want to me change the commit message to be more clear ? In the first case, I can write here a detailed explanation. In the second case I can make a longer commit message.
Or both?
I want all those call chains explained in the commit message, otherwise I have no easy way to confirm whether the change is correct or not.
Hmm. OK, so I had a bit of a dig around and seems that what we do now .detect()->drm_get_edid()->drm_connector_update_edid_property()->drm_add_display_info()
Yes. I said before that I felt something was wrong when I read the documentation and then the code. To be more explicit now, I expected that drm_connector_update_edid_property() will be done in the fill_modes/get_modes phase instead of when reading the edid. The documentation suggests that but the code reads the edid in the detect phase. Now, since drm_connector_update_edid_property() is called in the detect phase, it is not necessary to keep the edid data in the private connector struct. It is in struct drm_connector from the beginning. But this is topic for another patch.
Now the question is when did that start happening? Looks like it was commit 4b4df570b41d ("drm: Update edid-derived drm_display_info fields at edid property set [v2]") that started to call drm_add_display_info() from drm_connector_update_edid_property(), and then commit 5186421cbfe2 ("drm: Introduce epoch counter to drm_connector") started to call drm_connector_update_edid_property() from drm_get_edid(). Before both of those commits were in place display_info would still contain some stale garbage during .detect().
That is the story I think we want in these commit messages since it a) explains why the old code was directly parsing the edid instead b) why it's now safe to change this
------------------commit-message?--------------------
drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Commit a92d083d08b0 created the new flag is_hdmi in drm_display_info which is set when sink compliant with CEA-861 (EDID) shall be treated as an HDMI sink.
From that day, this value can be used in some cases instead of
calling drm_detect_hdmi_monitor() and a second parse is avoided because drm_detect_hdmi_monitor() parses. A TODO task was registered in Documentation/gpu/todo.rst to perform that task in the future.
The flag drm_display_info.is_hdmi is set in the function drm_add_display_info(), which is called from drm_connector_update_edid_property(). Since commit 5186421cbfe2, drm_get_edid() calls drm_connector_update_edid_property() when reading the edid data from an i2c adapter. Therefore, in these cases drm_display_info.is_hdmi is updated to its correct value when returning from drm_get_edid().
Replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi in the cases when drm_detect_hdmi_monitor() is called after a read from an i2c adapter using drm_get_edid() in the i915 driver. -----------------------------------------------
PS. connector->force handling in drm_get_edid() looks a bit busted since it doesn't call drm_connector_update_edid_property() at all in some cases. I think there might be some path that leads there anywya if/when we change connector->force, but we should fix drm_get_edid() to do the right thing regarless.
In those cases, the edid isn't read and NULL is returned by drm_get_edid(). No problem because display_info.is_hdmi is inside an if (edid != NULL).
BTW, struct intel_connector is allocated with kzalloc, so the initial value of is_hdmi is zero. The connector isn't HDMI by default.
BR. Claudio Suarez.
On Mon, Oct 25, 2021 at 12:17:37AM +0200, Claudio Suarez wrote: [...]
No new comments about this, I suppose everything is fine.
I'm going to send the patch with this changes. Thanks to all and special thanks to you, Ville. Hope this helps the kernel. Don't hesitate to ask new changes if necessary.
Best regards Claudio Suarez
Commit a92d083d08b0 created the new flag is_hdmi in drm_display_info which is set when sink compliant with CEA-861 (EDID) will be treated as an HDMI sink.
From that day, this value can be used in some cases instead of
calling drm_detect_hdmi_monitor() and a second parse is avoided because drm_detect_hdmi_monitor() parses. A TODO task was registered in Documentation/gpu/todo.rst to perform that task in the future.
The flag drm_display_info.is_hdmi is set in the function drm_add_display_info(), which is called from drm_connector_update_edid_property(). Since commit 5186421cbfe2, drm_get_edid() calls drm_connector_update_edid_property() when reading the edid data from an i2c adapter. Therefore, in these cases drm_display_info.is_hdmi is updated to its correct value when returning from drm_get_edid().
Replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi in the cases when drm_detect_hdmi_monitor() is called after a read from an i2c adapter using drm_get_edid() in the i915 driver.
Signed-off-by: Claudio Suarez cssk@net-c.es --- drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +- drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index b04685bb6439..008e5b0ba408 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2355,7 +2355,7 @@ intel_hdmi_set_edid(struct drm_connector *connector) to_intel_connector(connector)->detect_edid = edid; if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) { intel_hdmi->has_audio = drm_detect_monitor_audio(edid); - intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid); + intel_hdmi->has_hdmi_sink = connector->display_info.is_hdmi;
connected = true; } diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c index 6cb27599ea03..b4065e4df644 100644 --- a/drivers/gpu/drm/i915/display/intel_sdvo.c +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c @@ -2060,8 +2060,9 @@ intel_sdvo_tmds_sink_detect(struct drm_connector *connector) if (edid->input & DRM_EDID_INPUT_DIGITAL) { status = connector_status_connected; if (intel_sdvo_connector->is_hdmi) { - intel_sdvo->has_hdmi_monitor = drm_detect_hdmi_monitor(edid); intel_sdvo->has_hdmi_audio = drm_detect_monitor_audio(edid); + intel_sdvo->has_hdmi_monitor = + connector->display_info.is_hdmi; } } else status = connector_status_disconnected;
dri-devel@lists.freedesktop.org
-
Claudio Suarez -
Dmitry Baryshkov -
Inki Dae -
Ville Syrjälä