drm: conversions to struct drm_edid
Subsystems (16)
cover drm: conversions to struct drm_edid
Convert more drivers to struct drm_edid.
Compile tested only.
Jani Nikula (11):
drm/rockchip: cdn-dp: get rid of drm_edid_raw() drm/sti/sti_hdmi: convert to struct drm_edid drm/bridge: analogix_dp: convert to struct drm_edid drm/exynos: hdmi: convert to struct drm_edid drm/hisilicon/hibmc: convert to struct drm_edid drm/loongson/7a1000: convert to struct drm_edid drm/loongson/7a2000: convert to struct drm_edid drm/msm/dp: switch to struct drm_edid drm/tegra: convert to struct drm_edid drm/imx/tve: convert to struct drm_edid drm/imx/ldb: convert to struct drm_edid
.../drm/bridge/analogix/analogix_dp_core.c | 15 +++--- drivers/gpu/drm/exynos/exynos_hdmi.c | 23 +++++---- .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 17 ++++--- drivers/gpu/drm/imx/ipuv3/imx-ldb.c | 24 +++++----- drivers/gpu/drm/imx/ipuv3/imx-tve.c | 14 +++--- drivers/gpu/drm/loongson/lsdc_output_7a1000.c | 15 +++--- drivers/gpu/drm/loongson/lsdc_output_7a2000.c | 15 +++--- drivers/gpu/drm/msm/dp/dp_display.c | 11 ++--- drivers/gpu/drm/msm/dp/dp_panel.c | 47 ++++++------------- drivers/gpu/drm/msm/dp/dp_panel.h | 2 +- drivers/gpu/drm/rockchip/cdn-dp-core.c | 16 +++---- drivers/gpu/drm/sti/sti_hdmi.c | 24 ++++++---- drivers/gpu/drm/tegra/drm.h | 2 +- drivers/gpu/drm/tegra/output.c | 29 +++++++----- 14 files changed, 121 insertions(+), 133 deletions(-)
Signature · 1 line
2.39.2
PATCH 1/11 drm/rockchip: cdn-dp: get rid of drm_edid_raw()
The dimensions are available in display info, so there's no need for raw EDID access. While at it, move the debug logging to where the EDID is actually read.
---
--- drivers/gpu/drm/rockchip/cdn-dp-core.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)
drivers/gpu/drm/rockchip/cdn-dp-core.c +6 -10
@@ -266,15 +266,6 @@ static int cdn_dp_connector_get_modes(struct drm_connector *connector)mutex_lock (&dp->lock); -if (dp->drm_edid) { -/* FIXME: get rid of drm_edid_raw() */ -const struct edid *edid =drm_edid_raw (dp->drm_edid); - -DRM_DEV_DEBUG_KMS (dp->dev,"got edid: width[%d] x height[%d]\n" , - edid->width_cm, edid->height_cm); - - } - ret =drm_edid_connector_add_modes (connector);mutex_unlock (&dp->lock);
@@ -369,6 +360,7 @@ static int cdn_dp_firmware_init(struct cdn_dp_device *dp)static int cdn_dp_get_sink_capability (struct cdn_dp_device *dp) { +const struct drm_display_info *info = &dp->connector.display_info;int ret;if (!cdn_dp_check_sink_connection (dp))
@@ -386,7 +378,11 @@ static int cdn_dp_get_sink_capability(struct cdn_dp_device *dp) cdn_dp_get_edid_block, dp);drm_edid_connector_update (&dp->connector, dp->drm_edid); - dp->sink_has_audio = dp->connector.display_info.has_audio; + dp->sink_has_audio = info->has_audio; + +if (dp->drm_edid) +DRM_DEV_DEBUG_KMS (dp->dev,"got edid: width[%d] x height[%d]\n" , + info->width_mm /10 , info->height_mm /10 );return 0 ; }
Signature · 1 line
2.39.2
Re: [PATCH 01/11] drm/rockchip: cdn-dp: get rid of drm_edid_raw()
Quoted 8 lines · Jani Nikula
The dimensions are available in display info, so there's no need for raw EDID access. While at it, move the debug logging to where the EDID is actually read.
Signed-off-by Jani Nikula <[email protected]>---
Quoted 8 lines
Cc Sandy Huang <[email protected]>Cc "Heiko Stübner" <[email protected]>Cc Andy Yan <[email protected]>Cc Maarten Lankhorst <[email protected]>Cc Maxime Ripard <[email protected]>Cc Thomas Zimmermann <[email protected]>
Quoted 48 lines
--- drivers/gpu/drm/rockchip/cdn-dp-core.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)drivers/gpu/drm/rockchip/cdn-dp-core.c +6
-10@@ -266,15 +266,6 @@ static int cdn_dp_connector_get_modes(struct drm_connector *connector)mutex_lock (&dp->lock); -if (dp->drm_edid) { -/* FIXME: get rid of drm_edid_raw() */ -const struct edid *edid =drm_edid_raw (dp->drm_edid); - -DRM_DEV_DEBUG_KMS (dp->dev,"got edid: width[%d] x height[%d]\n" , - edid->width_cm, edid->height_cm); - - } - ret =drm_edid_connector_add_modes (connector);mutex_unlock (&dp->lock);@@ -369,6 +360,7 @@ static int cdn_dp_firmware_init(struct cdn_dp_device *dp)static int cdn_dp_get_sink_capability (struct cdn_dp_device *dp) { +const struct drm_display_info *info = &dp->connector.display_info;int ret;if (!cdn_dp_check_sink_connection (dp))@@ -386,7 +378,11 @@ static int cdn_dp_get_sink_capability(struct cdn_dp_device *dp) cdn_dp_get_edid_block, dp);drm_edid_connector_update (&dp->connector, dp->drm_edid); - dp->sink_has_audio = dp->connector.display_info.has_audio; + dp->sink_has_audio = info->has_audio; + +if (dp->drm_edid) +DRM_DEV_DEBUG_KMS (dp->dev,"got edid: width[%d] x height[%d]\n" , + info->width_mm /10 , info->height_mm /10 );return 0 ; }Signature · 1 line
2.39.2
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
PATCH 2/11 drm/sti/sti_hdmi: convert to struct drm_edid
In reply to Jani NikulaPrefer the struct drm_edid based functions for reading the EDID and updating the connector.
The functional change is that the CEC physical address gets invalidated when the EDID could not be read.
---
--- drivers/gpu/drm/sti/sti_hdmi.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)
drivers/gpu/drm/sti/sti_hdmi.c +14 -10
@@ -974,28 +974,32 @@ static const struct drm_bridge_funcs sti_hdmi_bridge_funcs = {static int sti_hdmi_connector_get_modes (struct drm_connector *connector) { +const struct drm_display_info *info = &connector->display_info;struct sti_hdmi_connector *hdmi_connector =to_sti_hdmi_connector (connector);struct sti_hdmi *hdmi = hdmi_connector->hdmi; -struct edid *edid; +const struct drm_edid *drm_edid;int count;DRM_DEBUG_DRIVER ("\n" ); - edid =drm_get_edid (connector, hdmi->ddc_adapt); -if (!edid) - goto fail; + drm_edid =drm_edid_read_ddc (connector, hdmi->ddc_adapt); + +drm_edid_connector_update (connector, drm_edid); -cec_notifier_set_phys_addr_from_edid (hdmi->notifier, edid); +cec_notifier_set_phys_addr (hdmi->notifier, + connector->display_info.source_physical_address); + +if (!drm_edid) + goto fail; - count =drm_add_edid_modes (connector, edid); -drm_connector_update_edid_property (connector, edid); + count =drm_edid_connector_add_modes (connector);DRM_DEBUG_KMS ("%s : %dx%d cm\n" , - (connector->display_info.is_hdmi ?"hdmi monitor" :"dvi monitor" ), - edid->width_cm, edid->height_cm); + info->is_hdmi ?"hdmi monitor" :"dvi monitor" , + info->width_mm /10 , info->height_mm /10 ); -kfree (edid); +drm_edid_free (drm_edid);return count; fail:
Signature · 1 line
2.39.2
Re: [PATCH 02/11] drm/sti/sti_hdmi: convert to struct drm_edid
Quoted 30 lines · Jani Nikula
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.The functional change is that the CEC physical address gets invalidated when the EDID could not be read.
Signed-off-by Jani Nikula <[email protected]>---
Cc Alain Volmat <[email protected]>Cc Maarten Lankhorst <[email protected]>Cc Maxime Ripard <[email protected]>Cc Thomas Zimmermann <[email protected]>--- drivers/gpu/drm/sti/sti_hdmi.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)drivers/gpu/drm/sti/sti_hdmi.c +1
-0@@ -974,28 +974,32 @@ static const struct drm_bridge_funcs sti_hdmi_bridge_funcs = {static int sti_hdmi_connector_get_modes (struct drm_connector *connector) { +const struct drm_display_info *info = &connector->display_info;struct sti_hdmi_connector *hdmi_connector =to_sti_hdmi_connector (connector);struct sti_hdmi *hdmi = hdmi_connector->hdmi;
drivers/gpu/drm/sti/sti_hdmi.c static const struct drm_bridge_funcs sti_hdmi_bridge_funcs = { · line 986
Quoted 10 lines
drivers/gpu/drm/sti/sti_hdmi.c +2
-4-struct edid *edid; +const struct drm_edid *drm_edid;int count;DRM_DEBUG_DRIVER ("\n" ); - edid =drm_get_edid (connector, hdmi->ddc_adapt); -if (!edid) - goto fail; + drm_edid =drm_edid_read_ddc (connector, hdmi->ddc_adapt);
I think you can use drm_edid_read here since the ddc is correctly set up with drm_connector_init_with_ddc()
drivers/gpu/drm/sti/sti_hdmi.c static const struct drm_bridge_funcs sti_hdmi_bridge_funcs = { · line 994
Quoted 9 lines
drivers/gpu/drm/sti/sti_hdmi.c +7
-1+ +drm_edid_connector_update (connector, drm_edid); -cec_notifier_set_phys_addr_from_edid (hdmi->notifier, edid); +cec_notifier_set_phys_addr (hdmi->notifier, + connector->display_info.source_physical_address); + +if (!drm_edid) + goto fail;
Unless I missed something, all the functions can cope with a NULL edid, but this jump means in the failure case you'll return stack garbage in count. Just drop this check?
Quoted 4 lines
drivers/gpu/drm/sti/sti_hdmi.c +1
-2- count =drm_add_edid_modes (connector, edid); -drm_connector_update_edid_property (connector, edid); + count =drm_edid_connector_add_modes (connector);
drivers/gpu/drm/sti/sti_hdmi.c static const struct drm_bridge_funcs sti_hdmi_bridge_funcs = { · line 1003
Quoted 10 lines
drivers/gpu/drm/sti/sti_hdmi.c +3
-3DRM_DEBUG_KMS ("%s : %dx%d cm\n" , - (connector->display_info.is_hdmi ?"hdmi monitor" :"dvi monitor" ), - edid->width_cm, edid->height_cm); + info->is_hdmi ?"hdmi monitor" :"dvi monitor" , + info->width_mm /10 , info->height_mm /10 ); -kfree (edid); +drm_edid_free (drm_edid);return count;
With the two items addressed:
Quoted 5 lines
drivers/gpu/drm/sti/sti_hdmi.c +0
-0fail:Signature · 1 line
2.39.2
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 02/11] drm/sti/sti_hdmi: convert to struct drm_edid
Quoted 48 lines · Daniel Vetter
Quoted 30 lines · Jani Nikula
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.The functional change is that the CEC physical address gets invalidated when the EDID could not be read.
Signed-off-by Jani Nikula <[email protected]>---
Cc Alain Volmat <[email protected]>Cc Maarten Lankhorst <[email protected]>Cc Maxime Ripard <[email protected]>Cc Thomas Zimmermann <[email protected]>--- drivers/gpu/drm/sti/sti_hdmi.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)drivers/gpu/drm/sti/sti_hdmi.c +1
-0@@ -974,28 +974,32 @@ static const struct drm_bridge_funcs sti_hdmi_bridge_funcs = {static int sti_hdmi_connector_get_modes (struct drm_connector *connector) { +const struct drm_display_info *info = &connector->display_info;struct sti_hdmi_connector *hdmi_connector =to_sti_hdmi_connector (connector);struct sti_hdmi *hdmi = hdmi_connector->hdmi;drivers/gpu/drm/sti/sti_hdmi.c static const struct drm_bridge_funcs sti_hdmi_bridge_funcs = { · line 986
Quoted 10 lines
drivers/gpu/drm/sti/sti_hdmi.c +2
-4-struct edid *edid; +const struct drm_edid *drm_edid;int count;DRM_DEBUG_DRIVER ("\n" ); - edid =drm_get_edid (connector, hdmi->ddc_adapt); -if (!edid) - goto fail; + drm_edid =drm_edid_read_ddc (connector, hdmi->ddc_adapt);I think you can use
drm_edid_readhere since the ddc is correctly set up withdrm_connector_init_with_ddc()Quoted 3 lines
drivers/gpu/drm/sti/sti_hdmi.c +2
-0+ +drm_edid_connector_update (connector, drm_edid);
Quoted 10 lines
drivers/gpu/drm/sti/sti_hdmi.c static const struct drm_bridge_funcs sti_hdmi_bridge_funcs = { · line 994
Quoted 6 lines
drivers/gpu/drm/sti/sti_hdmi.c +5
-1-cec_notifier_set_phys_addr_from_edid (hdmi->notifier, edid); +cec_notifier_set_phys_addr (hdmi->notifier, + connector->display_info.source_physical_address); + +if (!drm_edid) + goto fail;Unless I missed something, all the functions can cope with a NULL edid, but this jump means in the failure case you'll return stack garbage in count. Just drop this check?
Reviving an old thread... the goto fail path returns 0, not count.
BR,
Jani.
Quoted 25 lines
Quoted 5 lines
drivers/gpu/drm/sti/sti_hdmi.c +1
-2- count =drm_add_edid_modes (connector, edid); -drm_connector_update_edid_property (connector, edid); + count =drm_edid_connector_add_modes (connector);drivers/gpu/drm/sti/sti_hdmi.c static const struct drm_bridge_funcs sti_hdmi_bridge_funcs = { · line 1003
Quoted 10 lines
drivers/gpu/drm/sti/sti_hdmi.c +3
-3DRM_DEBUG_KMS ("%s : %dx%d cm\n" , - (connector->display_info.is_hdmi ?"hdmi monitor" :"dvi monitor" ), - edid->width_cm, edid->height_cm); + info->is_hdmi ?"hdmi monitor" :"dvi monitor" , + info->width_mm /10 , info->height_mm /10 ); -kfree (edid); +drm_edid_free (drm_edid);return count;With the two items addressed:
Reviewed-by Daniel Vetter <[email protected]>Quoted 5 lines
drivers/gpu/drm/sti/sti_hdmi.c +0
-0fail:Signature · 1 line
2.39.2
Signature · 1 line
Jani Nikula, Intel
Daniel Vetter
Reviewed-byRe: [PATCH 02/11] drm/sti/sti_hdmi: convert to struct drm_edid
Quoted 60 lines · Jani Nikula
Quoted 48 lines · Daniel Vetter
Quoted 30 lines · Jani Nikula
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.The functional change is that the CEC physical address gets invalidated when the EDID could not be read.
Signed-off-by Jani Nikula <[email protected]>---
Cc Alain Volmat <[email protected]>Cc Maarten Lankhorst <[email protected]>Cc Maxime Ripard <[email protected]>Cc Thomas Zimmermann <[email protected]>--- drivers/gpu/drm/sti/sti_hdmi.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)drivers/gpu/drm/sti/sti_hdmi.c +1
-0@@ -974,28 +974,32 @@ static const struct drm_bridge_funcs sti_hdmi_bridge_funcs = {static int sti_hdmi_connector_get_modes (struct drm_connector *connector) { +const struct drm_display_info *info = &connector->display_info;struct sti_hdmi_connector *hdmi_connector =to_sti_hdmi_connector (connector);struct sti_hdmi *hdmi = hdmi_connector->hdmi;drivers/gpu/drm/sti/sti_hdmi.c static const struct drm_bridge_funcs sti_hdmi_bridge_funcs = { · line 986
Quoted 10 lines
drivers/gpu/drm/sti/sti_hdmi.c +2
-4-struct edid *edid; +const struct drm_edid *drm_edid;int count;DRM_DEBUG_DRIVER ("\n" ); - edid =drm_get_edid (connector, hdmi->ddc_adapt); -if (!edid) - goto fail; + drm_edid =drm_edid_read_ddc (connector, hdmi->ddc_adapt);I think you can use
drm_edid_readhere since the ddc is correctly set up withdrm_connector_init_with_ddc()Quoted 3 lines
drivers/gpu/drm/sti/sti_hdmi.c +2
-0+ +drm_edid_connector_update (connector, drm_edid);Quoted 10 lines
drivers/gpu/drm/sti/sti_hdmi.c static const struct drm_bridge_funcs sti_hdmi_bridge_funcs = { · line 994
Quoted 6 lines
drivers/gpu/drm/sti/sti_hdmi.c +5
-1-cec_notifier_set_phys_addr_from_edid (hdmi->notifier, edid); +cec_notifier_set_phys_addr (hdmi->notifier, + connector->display_info.source_physical_address); + +if (!drm_edid) + goto fail;Unless I missed something, all the functions can cope with a NULL edid, but this jump means in the failure case you'll return stack garbage in count. Just drop this check?
Quoted 1 line
Reviving an old thread... the goto fail path returns 0, not count.
Oops, I stand corrected. With just drm_edid_read used:
Quoted 33 lines
BR,
Jani.Quoted 25 lines
Quoted 5 lines
drivers/gpu/drm/sti/sti_hdmi.c +1
-2- count =drm_add_edid_modes (connector, edid); -drm_connector_update_edid_property (connector, edid); + count =drm_edid_connector_add_modes (connector);drivers/gpu/drm/sti/sti_hdmi.c static const struct drm_bridge_funcs sti_hdmi_bridge_funcs = { · line 1003
Quoted 10 lines
drivers/gpu/drm/sti/sti_hdmi.c +3
-3DRM_DEBUG_KMS ("%s : %dx%d cm\n" , - (connector->display_info.is_hdmi ?"hdmi monitor" :"dvi monitor" ), - edid->width_cm, edid->height_cm); + info->is_hdmi ?"hdmi monitor" :"dvi monitor" , + info->width_mm /10 , info->height_mm /10 ); -kfree (edid); +drm_edid_free (drm_edid);return count;With the two items addressed:
Reviewed-by Daniel Vetter <[email protected]>Quoted 5 lines
drivers/gpu/drm/sti/sti_hdmi.c +0
-0fail:Signature · 1 line
2.39.2Signature · 1 line
Jani Nikula, Intel
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
PATCH 3/11 drm/bridge: analogix_dp: convert to struct drm_edid
In reply to Jani NikulaPrefer the struct drm_edid based functions for reading the EDID and updating the connector.
---
--- .../gpu/drm/bridge/analogix/analogix_dp_core.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +8 -7
@@ -1108,7 +1108,7 @@ static int analogix_dp_prepare_panel(struct analogix_dp_device *dp,static int analogix_dp_get_modes (struct drm_connector *connector) {struct analogix_dp_device *dp =to_dp (connector); -struct edid *edid; +const struct drm_edid *drm_edid;int ret, num_modes =0 ;if (dp->plat_data->panel) {
@@ -1120,12 +1120,13 @@ static int analogix_dp_get_modes(struct drm_connector *connector)return 0 ; } - edid =drm_get_edid (connector, &dp->aux.ddc); -if (edid) { -drm_connector_update_edid_property (&dp->connector, - edid); - num_modes +=drm_add_edid_modes (&dp->connector, edid); -kfree (edid); + drm_edid =drm_edid_read_ddc (connector, &dp->aux.ddc); + +drm_edid_connector_update (&dp->connector, drm_edid); + +if (drm_edid) { + num_modes +=drm_edid_connector_add_modes (&dp->connector); +drm_edid_free (drm_edid); } ret =analogix_dp_prepare_panel (dp, false, false);
Signature · 1 line
2.39.2
Re: [PATCH 03/11] drm/bridge: analogix_dp: convert to struct drm_edid
Quoted 16 lines · Jani Nikula
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.Signed-off-by Jani Nikula <[email protected]>---
Cc Andrzej Hajda <[email protected]>Cc Neil Armstrong <[email protected]>Cc Robert Foss <[email protected]>Cc Laurent Pinchart <[email protected]>Cc Jonas Karlman <[email protected]>Cc Jernej Skrabec <[email protected]>Cc Maarten Lankhorst <[email protected]>Cc Maxime Ripard <[email protected]>Cc Thomas Zimmermann <[email protected]>
Quoted 4 lines
--- .../gpu/drm/bridge/analogix/analogix_dp_core.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
Signature · 2 lines
With best wishes Dmitry
PATCH 4/11 drm/exynos: hdmi: convert to struct drm_edid
In reply to Jani NikulaPrefer the struct drm_edid based functions for reading the EDID and updating the connector.
The functional change is that the CEC physical address gets invalidated when the EDID could not be read.
---
--- drivers/gpu/drm/exynos/exynos_hdmi.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-)
drivers/gpu/drm/exynos/exynos_hdmi.c +13 -10
@@ -883,27 +883,30 @@ static const struct drm_connector_funcs hdmi_connector_funcs = {static int hdmi_get_modes (struct drm_connector *connector) {struct hdmi_context *hdata =connector_to_hdmi (connector); -struct edid *edid; +const struct drm_display_info *info = &connector->display_info; +const struct drm_edid *drm_edid;int ret;if (!hdata->ddc_adpt)return 0 ; - edid =drm_get_edid (connector, hdata->ddc_adpt); -if (!edid) + drm_edid =drm_edid_read_ddc (connector, hdata->ddc_adpt); + +drm_edid_connector_update (connector, drm_edid); + +cec_notifier_set_phys_addr (hdata->notifier, info->source_physical_address); + +if (!drm_edid)return 0 ; - hdata->dvi_mode = !connector->display_info.is_hdmi; + hdata->dvi_mode = !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); + info->width_mm /10 , info->height_mm /10 ); - ret =drm_add_edid_modes (connector, edid); + ret =drm_edid_connector_add_modes (connector); -kfree (edid); +drm_edid_free (drm_edid);return ret; }
Signature · 1 line
2.39.2
Re: [PATCH 04/11] drm/exynos: hdmi: convert to struct drm_edid
Hi, Jani Nikula,
Thanks for your contribution and sorry for being late. Below are my comments.
2024년 5월 14일 (화) 오후 9:57, Jani Nikula <[email protected]>님이 작성:
Quoted 31 lines
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.The functional change is that the CEC physical address gets invalidated when the EDID could not be read.
Signed-off-by Jani Nikula <[email protected]>---
Cc Inki Dae <[email protected]>Cc Seung-Woo Kim <[email protected]>Cc Kyungmin Park <[email protected]>Cc Krzysztof Kozlowski <[email protected]>Cc Alim Akhtar <[email protected]>--- drivers/gpu/drm/exynos/exynos_hdmi.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-)drivers/gpu/drm/exynos/exynos_hdmi.c +0
-1@@ -883,27 +883,30 @@ static const struct drm_connector_funcs hdmi_connector_funcs = {static int hdmi_get_modes (struct drm_connector *connector) {struct hdmi_context *hdata =connector_to_hdmi (connector); -struct edid *edid;
drivers/gpu/drm/exynos/exynos_hdmi.c static const struct drm_connector_funcs hdmi_connector_funcs = { · line 893
Quoted 10 lines
drivers/gpu/drm/exynos/exynos_hdmi.c +3
-2+const struct drm_display_info *info = &connector->display_info; +const struct drm_edid *drm_edid;int ret;if (!hdata->ddc_adpt)return 0 ; - edid =drm_get_edid (connector, hdata->ddc_adpt); -if (!edid) + drm_edid =drm_edid_read_ddc (connector, hdata->ddc_adpt);
drm_edid_read_ddc function can return NULL for an error. Could you add an exception handling?
drivers/gpu/drm/exynos/exynos_hdmi.c static const struct drm_connector_funcs hdmi_connector_funcs = { · line 895
Quoted 2 lines
drivers/gpu/drm/exynos/exynos_hdmi.c +2
-0+ +drm_edid_connector_update (connector, drm_edid);
Ditto. drm_edid_connector_update function can return a negative value for an error.
drivers/gpu/drm/exynos/exynos_hdmi.c static const struct drm_connector_funcs hdmi_connector_funcs = { · line 902
Quoted 8 lines
drivers/gpu/drm/exynos/exynos_hdmi.c +5
-1+ +cec_notifier_set_phys_addr (hdata->notifier, info->source_physical_address); + +if (!drm_edid)return 0 ; - hdata->dvi_mode = !connector->display_info.is_hdmi; + hdata->dvi_mode = !info->is_hdmi;
Above change wouldn't be related to this patch.
drivers/gpu/drm/exynos/exynos_hdmi.c static const struct drm_connector_funcs hdmi_connector_funcs = { · line 905
Quoted 7 lines
drivers/gpu/drm/exynos/exynos_hdmi.c +1
-4DRM_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); + info->width_mm /10 , info->height_mm /10 );
The purpose of this patch would be to replace edid with drm_edid so how about updating the above change like below?
drm_edid->edid->width_cm, erm_edid->edid->height_cm);
Thanks,
Inki Dae
Quoted 13 lines
drivers/gpu/drm/exynos/exynos_hdmi.c +2
-3- ret =drm_add_edid_modes (connector, edid); + ret =drm_edid_connector_add_modes (connector); -kfree (edid); +drm_edid_free (drm_edid);return ret; } --2.39.2
Re: [PATCH 04/11] drm/exynos: hdmi: convert to struct drm_edid
Quoted 43 lines · Inki Dae
Hi, Jani Nikula,
Thanks for your contribution and sorry for being late. Below are my comments.
2024년 5월 14일 (화) 오후 9:57, Jani Nikula <[email protected]>님이 작성:
Quoted 38 lines
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.The functional change is that the CEC physical address gets invalidated when the EDID could not be read.
Signed-off-by Jani Nikula <[email protected]>---
Cc Inki Dae <[email protected]>Cc Seung-Woo Kim <[email protected]>Cc Kyungmin Park <[email protected]>Cc Krzysztof Kozlowski <[email protected]>Cc Alim Akhtar <[email protected]>--- drivers/gpu/drm/exynos/exynos_hdmi.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-)drivers/gpu/drm/exynos/exynos_hdmi.c +2
-1@@ -883,27 +883,30 @@ static const struct drm_connector_funcs hdmi_connector_funcs = {static int hdmi_get_modes (struct drm_connector *connector) {struct hdmi_context *hdata =connector_to_hdmi (connector); -struct edid *edid; +const struct drm_display_info *info = &connector->display_info; +const struct drm_edid *drm_edid;int ret;if (!hdata->ddc_adpt)return 0 ;
Quoted 6 lines
drivers/gpu/drm/exynos/exynos_hdmi.c static const struct drm_connector_funcs hdmi_connector_funcs = { · line 893
Quoted 3 lines
drivers/gpu/drm/exynos/exynos_hdmi.c +1
-2- edid =drm_get_edid (connector, hdata->ddc_adpt); -if (!edid) + drm_edid =drm_edid_read_ddc (connector, hdata->ddc_adpt);
drm_edid_read_ddcfunction can return NULL for an error. Could you add an exception handling?
If it returns NULL, the below code intentionally passes NULL to drm_edid_connector_update() to reset the EDID. After that, cec_notifier_set_phys_addr() also resets the source physical address. And !drm_edid is handled after that.
Quoted 6 lines
drivers/gpu/drm/exynos/exynos_hdmi.c static const struct drm_connector_funcs hdmi_connector_funcs = { · line 895
Quoted 3 lines
drivers/gpu/drm/exynos/exynos_hdmi.c +2
-0+ +drm_edid_connector_update (connector, drm_edid);Ditto.
drm_edid_connector_updatefunction can return a negative value for an error.
Okay.cg
Quoted 11 lines
drivers/gpu/drm/exynos/exynos_hdmi.c static const struct drm_connector_funcs hdmi_connector_funcs = { · line 902
Quoted 9 lines
drivers/gpu/drm/exynos/exynos_hdmi.c +5
-1+ +cec_notifier_set_phys_addr (hdata->notifier, info->source_physical_address); + +if (!drm_edid)return 0 ; - hdata->dvi_mode = !connector->display_info.is_hdmi; + hdata->dvi_mode = !info->is_hdmi;Above change wouldn't be related to this patch.
Yeah, reuses info which is for info->source_physical_address which would be too long otherwise. Can add another intermediate patch for that.
Quoted 9 lines
drivers/gpu/drm/exynos/exynos_hdmi.c static const struct drm_connector_funcs hdmi_connector_funcs = { · line 905
Quoted 8 lines
drivers/gpu/drm/exynos/exynos_hdmi.c +1
-4DRM_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); + info->width_mm /10 , info->height_mm /10 );
Quoted 3 lines
The purpose of this patch would be to replace edid with
drm_edidso how about updating the above change like below?drm_edid->edid->width_cm, erm_edid->edid->height_cm);
struct drm_edid is an opaque type and drm_edid->edid is not available for drivers to look at.
BR,
Jani.
Quoted 17 lines
Thanks,
Inki DaeQuoted 13 lines
drivers/gpu/drm/exynos/exynos_hdmi.c +2
-3- ret =drm_add_edid_modes (connector, edid); + ret =drm_edid_connector_add_modes (connector); -kfree (edid); +drm_edid_free (drm_edid);return ret; } --2.39.2
Signature · 1 line
Jani Nikula, Intel
PATCH 5/11 drm/hisilicon/hibmc: convert to struct drm_edid
In reply to Jani NikulaPrefer the struct drm_edid based functions for reading the EDID and updating the connector.
---
--- .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c +10 -7
@@ -24,14 +24,16 @@static int hibmc_connector_get_modes (struct drm_connector *connector) { -int count; -void *edid;struct hibmc_connector *hibmc_connector =to_hibmc_connector (connector); +const struct drm_edid *drm_edid; +int count; + + drm_edid =drm_edid_read_ddc (connector, &hibmc_connector->adapter); - edid =drm_get_edid (connector, &hibmc_connector->adapter); -if (edid) { -drm_connector_update_edid_property (connector, edid); - count =drm_add_edid_modes (connector, edid); +drm_edid_connector_update (connector, drm_edid); + +if (drm_edid) { + count =drm_edid_connector_add_modes (connector);if (count) goto out; }
@@ -42,7 +44,8 @@ static int hibmc_connector_get_modes(struct drm_connector *connector)drm_set_preferred_mode (connector,1024 ,768 ); out: -kfree (edid); +drm_edid_free (drm_edid); +return count; }
Signature · 1 line
2.39.2
Thomas Zimmermann
maintainerRe: [PATCH 05/11] drm/hisilicon/hibmc: convert to struct drm_edid
Hi
Am 14.05.24 um 14:55 schrieb Jani Nikula:
Quoted 25 lines
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.Signed-off-by Jani Nikula <[email protected]>---
Cc Xinliang Liu <[email protected]>Cc Tian Tao <[email protected]>Cc Xinwei Kong <[email protected]>Cc Sumit Semwal <[email protected]>Cc Yongqin Liu <[email protected]>Cc John Stultz <[email protected]>Cc Maarten Lankhorst <[email protected]>Cc Maxime Ripard <[email protected]>Cc Thomas Zimmermann <[email protected]>--- .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c +0
-0
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c line 25
Quoted 2 lines
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c +0
-0@@ -24,14 +24,16 @@static int hibmc_connector_get_modes (struct drm_connector *connector)
Could this function be replaced with a call to drm_connector_helper_get_modes()? With the patch, the only difference is the call to the _noedid() function, which the DRM core does automatically. There will still be a difference in the maximum resolution, but standardizing on 1024x768 seems preferable to me.
Best regards
Thomas
Quoted 30 lines
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c +10
-7{ -int count; -void *edid;struct hibmc_connector *hibmc_connector =to_hibmc_connector (connector); +const struct drm_edid *drm_edid; +int count; + + drm_edid =drm_edid_read_ddc (connector, &hibmc_connector->adapter); - edid =drm_get_edid (connector, &hibmc_connector->adapter); -if (edid) { -drm_connector_update_edid_property (connector, edid); - count =drm_add_edid_modes (connector, edid); +drm_edid_connector_update (connector, drm_edid); + +if (drm_edid) { + count =drm_edid_connector_add_modes (connector);if (count) goto out; }@@ -42,7 +44,8 @@ static int hibmc_connector_get_modes(struct drm_connector *connector)drm_set_preferred_mode (connector,1024 ,768 ); out: -kfree (edid); +drm_edid_free (drm_edid); +return count; }
Signature · 7 lines
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
Re: [PATCH 05/11] drm/hisilicon/hibmc: convert to struct drm_edid
Quoted 29 lines · Thomas Zimmermann
Hi
Am 14.05.24 um 14:55 schrieb Jani Nikula:
Quoted 26 lines
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.Signed-off-by Jani Nikula <[email protected]>---
Cc Xinliang Liu <[email protected]>Cc Tian Tao <[email protected]>Cc Xinwei Kong <[email protected]>Cc Sumit Semwal <[email protected]>Cc Yongqin Liu <[email protected]>Cc John Stultz <[email protected]>Cc Maarten Lankhorst <[email protected]>Cc Maxime Ripard <[email protected]>Cc Thomas Zimmermann <[email protected]>--- .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c +0
-0@@ -24,14 +24,16 @@
Quoted 7 lines
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c line 25
Quoted 1 line
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c +0
-0static int hibmc_connector_get_modes (struct drm_connector *connector)Could this function be replaced with a call to
drm_connector_helper_get_modes()? With the patch, the only difference is the call to the_noedid()function, which the DRM core does automatically. There will still be a difference in the maximum resolution, but standardizing on 1024x768 seems preferable to me.
Looks like it might work, but personally, I'm trying to shy away from any further cleanups than just switching to struct drm_edid. I've got my plate pretty full already.
BR,
Jani.
Quoted 34 lines
Best regards
ThomasQuoted 30 lines
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c +10
-7{ -int count; -void *edid;struct hibmc_connector *hibmc_connector =to_hibmc_connector (connector); +const struct drm_edid *drm_edid; +int count; + + drm_edid =drm_edid_read_ddc (connector, &hibmc_connector->adapter); - edid =drm_get_edid (connector, &hibmc_connector->adapter); -if (edid) { -drm_connector_update_edid_property (connector, edid); - count =drm_add_edid_modes (connector, edid); +drm_edid_connector_update (connector, drm_edid); + +if (drm_edid) { + count =drm_edid_connector_add_modes (connector);if (count) goto out; }@@ -42,7 +44,8 @@ static int hibmc_connector_get_modes(struct drm_connector *connector)drm_set_preferred_mode (connector,1024 ,768 ); out: -kfree (edid); +drm_edid_free (drm_edid); +return count; }
Signature · 1 line
Jani Nikula, Intel
Re: [PATCH 05/11] drm/hisilicon/hibmc: convert to struct drm_edid
Hi
Am 15.05.24 um 14:34 schrieb Jani Nikula:
Quoted 36 lines · Thomas Zimmermann
Quoted 29 lines
Hi
Am 14.05.24 um 14:55 schrieb Jani Nikula:
Quoted 26 lines
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.Signed-off-by Jani Nikula <[email protected]>---
Cc Xinliang Liu <[email protected]>Cc Tian Tao <[email protected]>Cc Xinwei Kong <[email protected]>Cc Sumit Semwal <[email protected]>Cc Yongqin Liu <[email protected]>Cc John Stultz <[email protected]>Cc Maarten Lankhorst <[email protected]>Cc Maxime Ripard <[email protected]>Cc Thomas Zimmermann <[email protected]>--- .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c +0
-0@@ -24,14 +24,16 @@Quoted 6 lines
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c line 25
Quoted 1 line
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c +0
-0static int hibmc_connector_get_modes (struct drm_connector *connector)Could this function be replaced with a call to
drm_connector_helper_get_modes()? With the patch, the only difference is the call to the_noedid()function, which the DRM core does automatically. There will still be a difference in the maximum resolution, but standardizing on 1024x768 seems preferable to me.
Quoted 3 lines
Looks like it might work, but personally, I'm trying to shy away from any further cleanups than just switching to struct
drm_edid. I've got my plate pretty full already.
In that case
for the current patch.
Best regards
Thomas
Quoted 38 lines
BR,
Jani.Quoted 33 lines
Best regards
ThomasQuoted 30 lines
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c +10
-7{ -int count; -void *edid;struct hibmc_connector *hibmc_connector =to_hibmc_connector (connector); +const struct drm_edid *drm_edid; +int count; + + drm_edid =drm_edid_read_ddc (connector, &hibmc_connector->adapter); - edid =drm_get_edid (connector, &hibmc_connector->adapter); -if (edid) { -drm_connector_update_edid_property (connector, edid); - count =drm_add_edid_modes (connector, edid); +drm_edid_connector_update (connector, drm_edid); + +if (drm_edid) { + count =drm_edid_connector_add_modes (connector);if (count) goto out; }@@ -42,7 +44,8 @@ static int hibmc_connector_get_modes(struct drm_connector *connector)drm_set_preferred_mode (connector,1024 ,768 ); out: -kfree (edid); +drm_edid_free (drm_edid); +return count; }
Signature · 7 lines
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
PATCH 6/11 drm/loongson/7a1000: convert to struct drm_edid
In reply to Jani NikulaPrefer the struct drm_edid based functions for reading the EDID and updating the connector.
---
--- drivers/gpu/drm/loongson/lsdc_output_7a1000.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
drivers/gpu/drm/loongson/lsdc_output_7a1000.c +7 -8
@@ -40,16 +40,15 @@static int ls7a1000_dpi_connector_get_modes (struct drm_connector *conn) { -unsigned int num =0 ; -struct edid *edid; +int num;if (conn->ddc) { - edid =drm_get_edid (conn, conn->ddc); -if (edid) { -drm_connector_update_edid_property (conn, edid); - num =drm_add_edid_modes (conn, edid); -kfree (edid); - } +const struct drm_edid *drm_edid; + + drm_edid =drm_edid_read (conn); +drm_edid_connector_update (conn, drm_edid); + num =drm_edid_connector_add_modes (conn); +drm_edid_free (drm_edid);return num; }
Signature · 1 line
2.39.2
Re: [06/11] drm/loongson/7a1000: convert to struct drm_edid
Hi, Jani
I love your patch, thanks.
Quoted 5 lines · Jani Nikula
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.Signed-off-by Jani Nikula <[email protected]>---
Signature · 2 lines
Best regards, Sui
PATCH 7/11 drm/loongson/7a2000: convert to struct drm_edid
In reply to Jani NikulaPrefer the struct drm_edid based functions for reading the EDID and updating the connector.
---
--- drivers/gpu/drm/loongson/lsdc_output_7a2000.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
drivers/gpu/drm/loongson/lsdc_output_7a2000.c +7 -8
@@ -44,16 +44,15 @@static int ls7a2000_connector_get_modes (struct drm_connector *connector) { -unsigned int num =0 ; -struct edid *edid; +int num;if (connector->ddc) { - edid =drm_get_edid (connector, connector->ddc); -if (edid) { -drm_connector_update_edid_property (connector, edid); - num =drm_add_edid_modes (connector, edid); -kfree (edid); - } +const struct drm_edid *drm_edid; + + drm_edid =drm_edid_read (connector); +drm_edid_connector_update (connector, drm_edid); + num =drm_edid_connector_add_modes (connector); +drm_edid_free (drm_edid);return num; }
Signature · 1 line
2.39.2
Re: [07/11] drm/loongson/7a2000: convert to struct drm_edid
Hi, Jani
I love your patch, thanks.
Quoted 5 lines · Jani Nikula
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.Signed-off-by Jani Nikula <[email protected]>---
Signature · 2 lines
Best regards, Sui
PATCH 8/11 drm/msm/dp: switch to struct drm_edid
In reply to Jani NikulaPrefer the struct drm_edid based functions for reading the EDID and updating the connector.
Simplify the flow by updating the EDID property when the EDID is read instead of at .get_modes.
---
--- drivers/gpu/drm/msm/dp/dp_display.c | 11 +++---- drivers/gpu/drm/msm/dp/dp_panel.c | 47 +++++++++-------------------- drivers/gpu/drm/msm/dp/dp_panel.h | 2 +- 3 files changed, 20 insertions(+), 40 deletions(-)
drivers/gpu/drm/msm/dp/dp_display.c +5 -6
@@ -360,26 +360,25 @@ static int dp_display_send_hpd_notification(struct dp_display_private *dp,static int dp_display_process_hpd_high (struct dp_display_private *dp) { +struct drm_connector *connector = dp->dp_display.connector; +const struct drm_display_info *info = &connector->display_info;int rc =0 ; -struct edid *edid; - rc =dp_panel_read_sink_caps (dp->panel, dp->dp_display.connector); + rc =dp_panel_read_sink_caps (dp->panel, connector);if (rc) goto end;dp_link_process_request (dp->link);if (!dp->dp_display.is_edp) -drm_dp_set_subconnector_property (dp->dp_display.connector, +drm_dp_set_subconnector_property (connector, connector_status_connected, dp->panel->dpcd, dp->panel->downstream_ports); - edid = dp->panel->edid; - dp->dp_display.psr_supported = dp->panel->psr_cap.version && psr_enabled; - dp->audio_supported =drm_detect_monitor_audio (edid); + dp->audio_supported = info->has_audio;dp_panel_handle_sink_request (dp->panel);/*
drivers/gpu/drm/msm/dp/dp_panel.c +14 -33
82 lines not on this page · raw message
drivers/gpu/drm/msm/dp/dp_panel.h +1 -1
@@ -39,7 +39,7 @@ struct dp_panel {u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS ];struct dp_link_info link_info; -struct edid *edid; +const struct drm_edid *drm_edid;struct drm_connector *connector;struct dp_display_mode dp_mode;struct dp_panel_psr psr_cap;
Signature · 1 line
2.39.2
Re: [PATCH 08/11] drm/msm/dp: switch to struct drm_edid
Quoted 9 lines · Jani Nikula
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.Simplify the flow by updating the EDID property when the EDID is read instead of at .get_modes.
Signed-off-by Jani Nikula <[email protected]>---
The patch looks good to me, I'd like to hear an opinion from Doug (added to CC).
What is the merge strategy for the series? Do you plan to pick up all the patches to drm-misc or should we pick up individual patches into driver trees?
Quoted 13 lines
Cc Rob Clark <[email protected]>Cc Abhinav Kumar <[email protected]>Cc Dmitry Baryshkov <[email protected]>Cc Sean Paul <[email protected]>Cc Marijn Suijten <[email protected]>--- drivers/gpu/drm/msm/dp/dp_display.c | 11 +++---- drivers/gpu/drm/msm/dp/dp_panel.c | 47 +++++++++-------------------- drivers/gpu/drm/msm/dp/dp_panel.h | 2 +- 3 files changed, 20 insertions(+), 40 deletions(-)
[skipped]
drivers/gpu/drm/msm/dp/dp_panel.c void dp_panel_handle_sink_request(struct dp_panel *dp_panel) · line 231
Quoted 5 lines
drivers/gpu/drm/msm/dp/dp_panel.c +1
-0@@ -249,10 +228,12 @@ void dp_panel_handle_sink_request(struct dp_panel *dp_panel) panel = container_of(dp_panel, struct dp_panel_private, dp_panel);if (panel->link->sink_request &DP_TEST_LINK_EDID_READ ) { +/* FIXME: get rid of drm_edid_raw() */
The code here can get use of something like drm_edid_smth_checksum(). 'Something', because I could not come up with the word that would make it clear that it is the declared checksum instead of the actual / computed one.
Quoted 10 lines
drivers/gpu/drm/msm/dp/dp_panel.c +3
-2+const struct edid *edid =drm_edid_raw (dp_panel->drm_edid);u8 checksum; -if (dp_panel->edid) - checksum =dp_panel_get_edid_checksum (dp_panel->edid); +if (edid) + checksum =dp_panel_get_edid_checksum (edid);else checksum = dp_panel->connector->real_edid_checksum;
Signature · 2 lines
With best wishes Dmitry
Re: [PATCH 08/11] drm/msm/dp: switch to struct drm_edid
Quoted 7 lines · Dmitry Baryshkov
Quoted 6 lines · Jani Nikula
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.Simplify the flow by updating the EDID property when the EDID is read instead of at .get_modes.
Quoted 8 lines
Quoted 3 lines
Signed-off-by Jani Nikula <[email protected]>---
The patch looks good to me, I'd like to hear an opinion from Doug (added to CC).
Reviewed-by Dmitry Baryshkov <[email protected]>
Thanks!
Quoted 3 lines
What is the merge strategy for the series? Do you plan to pick up all the patches to drm-misc or should we pick up individual patches into driver trees?
I think all of the patches here are connected in theme, but independent. Either way is fine by me.
Quoted 21 lines
Quoted 15 lines
Cc Rob Clark <[email protected]>Cc Abhinav Kumar <[email protected]>Cc Dmitry Baryshkov <[email protected]>Cc Sean Paul <[email protected]>Cc Marijn Suijten <[email protected]>--- drivers/gpu/drm/msm/dp/dp_display.c | 11 +++---- drivers/gpu/drm/msm/dp/dp_panel.c | 47 +++++++++-------------------- drivers/gpu/drm/msm/dp/dp_panel.h | 2 +- 3 files changed, 20 insertions(+), 40 deletions(-)[skipped]
Quoted 3 lines
drivers/gpu/drm/msm/dp/dp_panel.c +0
-0@@ -249,10 +228,12 @@ void dp_panel_handle_sink_request(struct dp_panel *dp_panel) panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
Quoted 7 lines
drivers/gpu/drm/msm/dp/dp_panel.c void dp_panel_handle_sink_request(struct dp_panel *dp_panel) · line 231
Quoted 2 lines
drivers/gpu/drm/msm/dp/dp_panel.c +1
-0if (panel->link->sink_request &DP_TEST_LINK_EDID_READ ) { +/* FIXME: get rid of drm_edid_raw() */ The code here can get use of something like
drm_edid_smth_checksum(). 'Something', because I could not come up with the word that would make it clear that it is the declared checksum instead of the actual / computed one.
This is an annoying one, to be honest, and linked to the historical fact that we filter some EDID blocks that have an incorrect checksum.
(Some blocks, yes. We don't filter all blocks, because there are some nasty docks out there that modify the CTA block but fail to update the checksum, and filtering the CTA blocks would render the display useless. So we accept CTA blocks with incorrect checksums. But reject others. Yay.)
IMO the real fix would be to stop mucking with the EDID, and just expose it to userspace, warts and all. We could still ignore the EDID blocks with incorrect checksum while using it ourselves if we want to. And with that, we could just have a function that checks the last EDID block's checksum, and stop using this ->real_edid_checksum thing.
Anyway, yes, we could add the function already.
BR,
Jani.
Quoted 11 lines
Quoted 11 lines
drivers/gpu/drm/msm/dp/dp_panel.c +3
-2+const struct edid *edid =drm_edid_raw (dp_panel->drm_edid);u8 checksum; -if (dp_panel->edid) - checksum =dp_panel_get_edid_checksum (dp_panel->edid); +if (edid) + checksum =dp_panel_get_edid_checksum (edid);else checksum = dp_panel->connector->real_edid_checksum;
Signature · 1 line
Jani Nikula, Intel
Re: [PATCH 08/11] drm/msm/dp: switch to struct drm_edid
Quoted 56 lines · Jani Nikula
Quoted 7 lines · Dmitry Baryshkov
Quoted 6 lines · Jani Nikula
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.Simplify the flow by updating the EDID property when the EDID is read instead of at .get_modes.
Quoted 8 lines
Quoted 3 lines
Signed-off-by Jani Nikula <[email protected]>---
The patch looks good to me, I'd like to hear an opinion from Doug (added to CC).
Reviewed-by Dmitry Baryshkov <[email protected]>Thanks!
Quoted 3 lines
What is the merge strategy for the series? Do you plan to pick up all the patches to drm-misc or should we pick up individual patches into driver trees?
I think all of the patches here are connected in theme, but independent. Either way is fine by me.
Quoted 21 lines
Quoted 15 lines
Cc Rob Clark <[email protected]>Cc Abhinav Kumar <[email protected]>Cc Dmitry Baryshkov <[email protected]>Cc Sean Paul <[email protected]>Cc Marijn Suijten <[email protected]>--- drivers/gpu/drm/msm/dp/dp_display.c | 11 +++---- drivers/gpu/drm/msm/dp/dp_panel.c | 47 +++++++++-------------------- drivers/gpu/drm/msm/dp/dp_panel.h | 2 +- 3 files changed, 20 insertions(+), 40 deletions(-)[skipped]
Quoted 3 lines
drivers/gpu/drm/msm/dp/dp_panel.c +0
-0@@ -249,10 +228,12 @@ void dp_panel_handle_sink_request(struct dp_panel *dp_panel) panel = container_of(dp_panel, struct dp_panel_private, dp_panel);Quoted 7 lines
drivers/gpu/drm/msm/dp/dp_panel.c void dp_panel_handle_sink_request(struct dp_panel *dp_panel) · line 231
Quoted 2 lines
drivers/gpu/drm/msm/dp/dp_panel.c +1
-0if (panel->link->sink_request &DP_TEST_LINK_EDID_READ ) { +/* FIXME: get rid of drm_edid_raw() */ The code here can get use of something like
drm_edid_smth_checksum(). 'Something', because I could not come up with the word that would make it clear that it is the declared checksum instead of the actual / computed one.
Quoted 2 lines
This is an annoying one, to be honest, and linked to the historical fact that we filter some EDID blocks that have an incorrect checksum.
It is a part of the DP test suite if I remember correctly.
Quoted 32 lines
(Some blocks, yes. We don't filter all blocks, because there are some nasty docks out there that modify the CTA block but fail to update the checksum, and filtering the CTA blocks would render the display useless. So we accept CTA blocks with incorrect checksums. But reject others. Yay.)
IMO the real fix would be to stop mucking with the EDID, and just expose it to userspace, warts and all. We could still ignore the EDID blocks with incorrect checksum while using it ourselves if we want to. And with that, we could just have a function that checks the last EDID block's checksum, and stop using this ->real_edid_checksum thing.
Anyway, yes, we could add the function already.
BR,
Jani.Quoted 11 lines
drivers/gpu/drm/msm/dp/dp_panel.c void dp_panel_handle_sink_request(struct dp_panel *dp_panel) · line 239
Quoted 11 lines
drivers/gpu/drm/msm/dp/dp_panel.c +3
-2+const struct edid *edid =drm_edid_raw (dp_panel->drm_edid);u8 checksum; -if (dp_panel->edid) - checksum =dp_panel_get_edid_checksum (dp_panel->edid); +if (edid) + checksum =dp_panel_get_edid_checksum (edid);else checksum = dp_panel->connector->real_edid_checksum;--
Jani Nikula, Intel
Signature · 2 lines
With best wishes Dmitry
Doug Anderson
Reviewed-byRe: [PATCH 08/11] drm/msm/dp: switch to struct drm_edid
In reply to Dmitry BaryshkovHi,
Quoted 12 lines · Dmitry Baryshkov
Quoted 9 lines · Jani Nikula
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.Simplify the flow by updating the EDID property when the EDID is read instead of at .get_modes.
Signed-off-by Jani Nikula <[email protected]>---
Quoted 8 lines
The patch looks good to me, I'd like to hear an opinion from Doug (added to CC).
Reviewed-by Dmitry Baryshkov <[email protected]>What is the merge strategy for the series? Do you plan to pick up all the patches to drm-misc or should we pick up individual patches into driver trees?
I'm not sure I have too much to add here aside from what you guys have already talked about. I'm OK with:
Re: [PATCH 08/11] drm/msm/dp: switch to struct drm_edid
Quoted 21 lines · Doug Anderson
Hi,
Quoted 17 lines · Dmitry Baryshkov
Quoted 9 lines · Jani Nikula
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.Simplify the flow by updating the EDID property when the EDID is read instead of at .get_modes.
Signed-off-by Jani Nikula <[email protected]>---
The patch looks good to me, I'd like to hear an opinion from Doug (added to CC).
Reviewed-by Dmitry Baryshkov <[email protected]>
Quoted 8 lines
Quoted 3 lines
What is the merge strategy for the series? Do you plan to pick up all the patches to drm-misc or should we pick up individual patches into driver trees?
I'm not sure I have too much to add here aside from what you guys have already talked about. I'm OK with:
Reviewed-by Douglas Anderson <[email protected]>
I assume you'll want to pick this up for msm instead of me merging to drm-misc.
BR,
Jani.
Signature · 1 line
Jani Nikula, Intel
Re: [PATCH 08/11] drm/msm/dp: switch to struct drm_edid
Quoted 27 lines · Jani Nikula
Quoted 25 lines · Doug Anderson
Hi,
Quoted 20 lines · Dmitry Baryshkov
Quoted 9 lines · Jani Nikula
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.Simplify the flow by updating the EDID property when the EDID is read instead of at .get_modes.
Signed-off-by Jani Nikula <[email protected]>---
The patch looks good to me, I'd like to hear an opinion from Doug (added to CC).
Reviewed-by Dmitry Baryshkov <[email protected]>What is the merge strategy for the series? Do you plan to pick up all the patches to drm-misc or should we pick up individual patches into driver trees?
Quoted 7 lines
Quoted 4 lines
I'm not sure I have too much to add here aside from what you guys have already talked about. I'm OK with:
Reviewed-by Douglas Anderson <[email protected]>I assume you'll want to pick this up for msm instead of me merging to drm-misc.
Yes, it's on my todo list.
Signature · 2 lines
With best wishes Dmitry
PATCH 9/11 drm/tegra: convert to struct drm_edid
In reply to Jani NikulaPrefer the struct drm_edid based functions for reading the EDID and updating the connector.
---
--- drivers/gpu/drm/tegra/drm.h | 2 +- drivers/gpu/drm/tegra/output.c | 29 +++++++++++++++++------------ 2 files changed, 18 insertions(+), 13 deletions(-)
drivers/gpu/drm/tegra/drm.h +1 -1
@@ -133,7 +133,7 @@ struct tegra_output {struct drm_bridge *bridge;struct drm_panel *panel;struct i2c_adapter *ddc; -const struct edid *edid; +const struct drm_edid *drm_edid;struct cec_notifier *cec;unsigned int hpd_irq;struct gpio_desc *hpd_gpio;
drivers/gpu/drm/tegra/output.c +17 -12
@@ -21,7 +21,7 @@int tegra_output_connector_get_modes (struct drm_connector *connector) {struct tegra_output *output =connector_to_output (connector); -struct edid *edid =NULL ; +const struct drm_edid *drm_edid;int err =0 ;/*
@@ -34,18 +34,17 @@ int tegra_output_connector_get_modes(struct drm_connector *connector)return err; } -if (output->edid) - edid =kmemdup (output->edid,sizeof (*edid),GFP_KERNEL ); +if (output->drm_edid) + drm_edid =drm_edid_dup (output->drm_edid);else if (output->ddc) - edid =drm_get_edid (connector, output->ddc); + drm_edid =drm_edid_read_ddc (connector, output->ddc); -cec_notifier_set_phys_addr_from_edid (output->cec, edid); -drm_connector_update_edid_property (connector, edid); +drm_edid_connector_update (connector, drm_edid); +cec_notifier_set_phys_addr (output->cec, + connector->display_info.source_physical_address); -if (edid) { - err =drm_add_edid_modes (connector, edid); -kfree (edid); - } + err =drm_edid_connector_add_modes (connector); +drm_edid_free (drm_edid);return err; }
@@ -98,6 +97,7 @@ static irqreturn_t hpd_irq(int irq, void *data)int tegra_output_probe (struct tegra_output *output) {struct device_node *ddc, *panel; +const void *edid;unsigned long flags;int err, size;
@@ -124,8 +124,6 @@ int tegra_output_probe(struct tegra_output *output)return PTR_ERR (output->panel); } - output->edid =of_get_property (output->of_node,"nvidia,edid" , &size); - ddc =of_parse_phandle (output->of_node,"nvidia,ddc-i2c-bus" ,0 );if (ddc) { output->ddc =of_get_i2c_adapter_by_node (ddc);
@@ -137,6 +135,9 @@ int tegra_output_probe(struct tegra_output *output) } } + edid =of_get_property (output->of_node,"nvidia,edid" , &size); + output->drm_edid =drm_edid_alloc (edid, size); + output->hpd_gpio = devm_fwnode_gpiod_get(output->dev, of_fwnode_handle(output->of_node), "nvidia,hpd",
@@ -187,6 +188,8 @@ int tegra_output_probe(struct tegra_output *output)if (output->ddc)i2c_put_adapter (output->ddc); +drm_edid_free (output->drm_edid); +return err; }
@@ -197,6 +200,8 @@ void tegra_output_remove(struct tegra_output *output)if (output->ddc)i2c_put_adapter (output->ddc); + +drm_edid_free (output->drm_edid); }int tegra_output_init (struct drm_device *drm,struct tegra_output *output)
Signature · 1 line
2.39.2
Re: [PATCH 09/11] drm/tegra: convert to struct drm_edid
Quoted 43 lines · Jani Nikula
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.Signed-off-by Jani Nikula <[email protected]>---
Cc Thierry Reding <[email protected]>Cc Mikko Perttunen <[email protected]>Cc Jonathan Hunter <[email protected]>--- drivers/gpu/drm/tegra/drm.h | 2 +- drivers/gpu/drm/tegra/output.c | 29 +++++++++++++++++------------ 2 files changed, 18 insertions(+), 13 deletions(-)drivers/gpu/drm/tegra/drm.h +1
-1@@ -133,7 +133,7 @@ struct tegra_output {struct drm_bridge *bridge;struct drm_panel *panel;struct i2c_adapter *ddc; -const struct edid *edid; +const struct drm_edid *drm_edid;struct cec_notifier *cec;unsigned int hpd_irq;struct gpio_desc *hpd_gpio;drivers/gpu/drm/tegra/output.c +1
-1@@ -21,7 +21,7 @@int tegra_output_connector_get_modes (struct drm_connector *connector) {struct tegra_output *output =connector_to_output (connector); -struct edid *edid =NULL ; +const struct drm_edid *drm_edid;int err =0 ;/*
drivers/gpu/drm/tegra/output.c int tegra_output_connector_get_modes(struct drm_connector *connector) · line 40
Quoted 10 lines
drivers/gpu/drm/tegra/output.c +3
-3@@ -34,18 +34,17 @@ int tegra_output_connector_get_modes(struct drm_connector *connector)return err; } -if (output->edid) - edid =kmemdup (output->edid,sizeof (*edid),GFP_KERNEL ); +if (output->drm_edid) + drm_edid =drm_edid_dup (output->drm_edid);else if (output->ddc) - edid =drm_get_edid (connector, output->ddc); + drm_edid =drm_edid_read_ddc (connector, output->ddc);
Annoyingly the dsi output is the only one not using drm_connector_init_with_ddc() despite that I think it could, so we can't yet use drm_edid_read here ...
Quoted 25 lines
drivers/gpu/drm/tegra/output.c +6
-6-cec_notifier_set_phys_addr_from_edid (output->cec, edid); -drm_connector_update_edid_property (connector, edid); +drm_edid_connector_update (connector, drm_edid); +cec_notifier_set_phys_addr (output->cec, + connector->display_info.source_physical_address); -if (edid) { - err =drm_add_edid_modes (connector, edid); -kfree (edid); - } + err =drm_edid_connector_add_modes (connector); +drm_edid_free (drm_edid);return err; }@@ -98,6 +97,7 @@ static irqreturn_t hpd_irq(int irq, void *data)int tegra_output_probe (struct tegra_output *output) {struct device_node *ddc, *panel; +const void *edid;unsigned long flags;int err, size;
drivers/gpu/drm/tegra/output.c int tegra_output_probe(struct tegra_output *output) · line 128
Quoted 5 lines
drivers/gpu/drm/tegra/output.c +0
-2@@ -124,8 +124,6 @@ int tegra_output_probe(struct tegra_output *output)return PTR_ERR (output->panel); } - output->edid =of_get_property (output->of_node,"nvidia,edid" , &size); -
I think you can/should also delete tegra_output.edid completely, since not doing that freaked me out a bit until I checked everything :-) At least if I didn't git grep the wrong regex.
With that bikeshed addressed:
Quoted 34 lines
drivers/gpu/drm/tegra/output.c +7
-0ddc =of_parse_phandle (output->of_node,"nvidia,ddc-i2c-bus" ,0 );if (ddc) { output->ddc =of_get_i2c_adapter_by_node (ddc);@@ -137,6 +135,9 @@ int tegra_output_probe(struct tegra_output *output) } } + edid =of_get_property (output->of_node,"nvidia,edid" , &size); + output->drm_edid =drm_edid_alloc (edid, size); + output->hpd_gpio = devm_fwnode_gpiod_get(output->dev, of_fwnode_handle(output->of_node), "nvidia,hpd",@@ -187,6 +188,8 @@ int tegra_output_probe(struct tegra_output *output)if (output->ddc)i2c_put_adapter (output->ddc); +drm_edid_free (output->drm_edid); +return err; }@@ -197,6 +200,8 @@ void tegra_output_remove(struct tegra_output *output)if (output->ddc)i2c_put_adapter (output->ddc); + +drm_edid_free (output->drm_edid); }int tegra_output_init (struct drm_device *drm,struct tegra_output *output)Signature · 1 line
2.39.2
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 09/11] drm/tegra: convert to struct drm_edid
Quoted 17 lines · Daniel Vetter
Quoted 16 lines · Jani Nikula
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.Signed-off-by Jani Nikula <[email protected]>---
Cc Thierry Reding <[email protected]>Cc Mikko Perttunen <[email protected]>Cc Jonathan Hunter <[email protected]>--- drivers/gpu/drm/tegra/drm.h | 2 +- drivers/gpu/drm/tegra/output.c | 29 +++++++++++++++++------------ 2 files changed, 18 insertions(+), 13 deletions(-)
Quoted 10 lines
drivers/gpu/drm/tegra/drm.h struct tegra_output { · line 136
Quoted 10 lines
drivers/gpu/drm/tegra/drm.h +1
-1@@ -133,7 +133,7 @@ struct tegra_output {struct drm_bridge *bridge;struct drm_panel *panel;struct i2c_adapter *ddc; -const struct edid *edid; +const struct drm_edid *drm_edid;
I keep reviving old stuff, sorry.
Please take note here, removing the edid member.
Quoted 60 lines
Quoted 17 lines
drivers/gpu/drm/tegra/drm.h +0
-0struct cec_notifier *cec;unsigned int hpd_irq;struct gpio_desc *hpd_gpio;drivers/gpu/drm/tegra/output.c +1
-1@@ -21,7 +21,7 @@int tegra_output_connector_get_modes (struct drm_connector *connector) {struct tegra_output *output =connector_to_output (connector); -struct edid *edid =NULL ; +const struct drm_edid *drm_edid;int err =0 ;/* drivers/gpu/drm/tegra/output.c int tegra_output_connector_get_modes(struct drm_connector *connector) · line 40
Quoted 10 lines
drivers/gpu/drm/tegra/output.c +3
-3@@ -34,18 +34,17 @@ int tegra_output_connector_get_modes(struct drm_connector *connector)return err; } -if (output->edid) - edid =kmemdup (output->edid,sizeof (*edid),GFP_KERNEL ); +if (output->drm_edid) + drm_edid =drm_edid_dup (output->drm_edid);else if (output->ddc) - edid =drm_get_edid (connector, output->ddc); + drm_edid =drm_edid_read_ddc (connector, output->ddc);Annoyingly the dsi output is the only one not using
drm_connector_init_with_ddc()despite that I think it could, so we can't yet usedrm_edid_readhere ...Quoted 28 lines
drivers/gpu/drm/tegra/output.c +6
-6-cec_notifier_set_phys_addr_from_edid (output->cec, edid); -drm_connector_update_edid_property (connector, edid); +drm_edid_connector_update (connector, drm_edid); +cec_notifier_set_phys_addr (output->cec, + connector->display_info.source_physical_address); -if (edid) { - err =drm_add_edid_modes (connector, edid); -kfree (edid); - } + err =drm_edid_connector_add_modes (connector); +drm_edid_free (drm_edid);return err; }@@ -98,6 +97,7 @@ static irqreturn_t hpd_irq(int irq, void *data)int tegra_output_probe (struct tegra_output *output) {struct device_node *ddc, *panel; +const void *edid;unsigned long flags;int err, size;@@ -124,8 +124,6 @@ int tegra_output_probe(struct tegra_output *output)return PTR_ERR (output->panel); }
Quoted 6 lines
drivers/gpu/drm/tegra/output.c int tegra_output_probe(struct tegra_output *output) · line 128
Quoted 2 lines
drivers/gpu/drm/tegra/output.c +0
-2- output->edid =of_get_property (output->of_node,"nvidia,edid" , &size); -I think you can/should also delete tegra_output.edid completely, since not doing that freaked me out a bit until I checked everything :-) At least if I didn't git grep the wrong regex.
I thought I am removing tegra_output.edid. What am I missing?
Quoted 4 lines
With that bikeshed addressed:
Reviewed-by Daniel Vetter <[email protected]>
Thanks,
Jani.
Quoted 35 lines
Quoted 35 lines
drivers/gpu/drm/tegra/output.c +7
-0ddc =of_parse_phandle (output->of_node,"nvidia,ddc-i2c-bus" ,0 );if (ddc) { output->ddc =of_get_i2c_adapter_by_node (ddc);@@ -137,6 +135,9 @@ int tegra_output_probe(struct tegra_output *output) } } + edid =of_get_property (output->of_node,"nvidia,edid" , &size); + output->drm_edid =drm_edid_alloc (edid, size); + output->hpd_gpio = devm_fwnode_gpiod_get(output->dev, of_fwnode_handle(output->of_node), "nvidia,hpd",@@ -187,6 +188,8 @@ int tegra_output_probe(struct tegra_output *output)if (output->ddc)i2c_put_adapter (output->ddc); +drm_edid_free (output->drm_edid); +return err; }@@ -197,6 +200,8 @@ void tegra_output_remove(struct tegra_output *output)if (output->ddc)i2c_put_adapter (output->ddc); + +drm_edid_free (output->drm_edid); }int tegra_output_init (struct drm_device *drm,struct tegra_output *output)Signature · 1 line
2.39.2
Signature · 1 line
Jani Nikula, Intel
Re: [PATCH 09/11] drm/tegra: convert to struct drm_edid
Quoted 93 lines · Jani Nikula
Quoted 17 lines · Daniel Vetter
Quoted 16 lines · Jani Nikula
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.Signed-off-by Jani Nikula <[email protected]>---
Cc Thierry Reding <[email protected]>Cc Mikko Perttunen <[email protected]>Cc Jonathan Hunter <[email protected]>--- drivers/gpu/drm/tegra/drm.h | 2 +- drivers/gpu/drm/tegra/output.c | 29 +++++++++++++++++------------ 2 files changed, 18 insertions(+), 13 deletions(-)Quoted 10 lines
drivers/gpu/drm/tegra/drm.h struct tegra_output { · line 136
Quoted 10 lines
drivers/gpu/drm/tegra/drm.h +1
-1@@ -133,7 +133,7 @@ struct tegra_output {struct drm_bridge *bridge;struct drm_panel *panel;struct i2c_adapter *ddc; -const struct edid *edid; +const struct drm_edid *drm_edid;I keep reviving old stuff, sorry.
Please take note here, removing the edid member.
Quoted 60 lines
Quoted 17 lines
drivers/gpu/drm/tegra/drm.h +0
-0struct cec_notifier *cec;unsigned int hpd_irq;struct gpio_desc *hpd_gpio;drivers/gpu/drm/tegra/output.c +1
-1@@ -21,7 +21,7 @@int tegra_output_connector_get_modes (struct drm_connector *connector) {struct tegra_output *output =connector_to_output (connector); -struct edid *edid =NULL ; +const struct drm_edid *drm_edid;int err =0 ;/* drivers/gpu/drm/tegra/output.c int tegra_output_connector_get_modes(struct drm_connector *connector) · line 40
Quoted 10 lines
drivers/gpu/drm/tegra/output.c +3
-3@@ -34,18 +34,17 @@ int tegra_output_connector_get_modes(struct drm_connector *connector)return err; } -if (output->edid) - edid =kmemdup (output->edid,sizeof (*edid),GFP_KERNEL ); +if (output->drm_edid) + drm_edid =drm_edid_dup (output->drm_edid);else if (output->ddc) - edid =drm_get_edid (connector, output->ddc); + drm_edid =drm_edid_read_ddc (connector, output->ddc);Annoyingly the dsi output is the only one not using
drm_connector_init_with_ddc()despite that I think it could, so we can't yet usedrm_edid_readhere ...Quoted 28 lines
drivers/gpu/drm/tegra/output.c +6
-6-cec_notifier_set_phys_addr_from_edid (output->cec, edid); -drm_connector_update_edid_property (connector, edid); +drm_edid_connector_update (connector, drm_edid); +cec_notifier_set_phys_addr (output->cec, + connector->display_info.source_physical_address); -if (edid) { - err =drm_add_edid_modes (connector, edid); -kfree (edid); - } + err =drm_edid_connector_add_modes (connector); +drm_edid_free (drm_edid);return err; }@@ -98,6 +97,7 @@ static irqreturn_t hpd_irq(int irq, void *data)int tegra_output_probe (struct tegra_output *output) {struct device_node *ddc, *panel; +const void *edid;unsigned long flags;int err, size;@@ -124,8 +124,6 @@ int tegra_output_probe(struct tegra_output *output)return PTR_ERR (output->panel); }
Quoted 8 lines
Quoted 6 lines
drivers/gpu/drm/tegra/output.c int tegra_output_probe(struct tegra_output *output) · line 128
Quoted 2 lines
drivers/gpu/drm/tegra/output.c +0
-2- output->edid =of_get_property (output->of_node,"nvidia,edid" , &size); -I think you can/should also delete tegra_output.edid completely, since not doing that freaked me out a bit until I checked everything :-) At least if I didn't git grep the wrong regex.
I thought I am removing tegra_output.edid. What am I missing?
I'm sometimes just blind. rb without bikeshed applies, sorry for the confusion.
-Sima
Quoted 48 lines
Quoted 5 lines
With that bikeshed addressed:
Reviewed-by Daniel Vetter <[email protected]>Thanks,
Jani.Quoted 35 lines
Quoted 35 lines
drivers/gpu/drm/tegra/output.c +7
-0ddc =of_parse_phandle (output->of_node,"nvidia,ddc-i2c-bus" ,0 );if (ddc) { output->ddc =of_get_i2c_adapter_by_node (ddc);@@ -137,6 +135,9 @@ int tegra_output_probe(struct tegra_output *output) } } + edid =of_get_property (output->of_node,"nvidia,edid" , &size); + output->drm_edid =drm_edid_alloc (edid, size); + output->hpd_gpio = devm_fwnode_gpiod_get(output->dev, of_fwnode_handle(output->of_node), "nvidia,hpd",@@ -187,6 +188,8 @@ int tegra_output_probe(struct tegra_output *output)if (output->ddc)i2c_put_adapter (output->ddc); +drm_edid_free (output->drm_edid); +return err; }@@ -197,6 +200,8 @@ void tegra_output_remove(struct tegra_output *output)if (output->ddc)i2c_put_adapter (output->ddc); + +drm_edid_free (output->drm_edid); }int tegra_output_init (struct drm_device *drm,struct tegra_output *output)Signature · 1 line
2.39.2Signature · 1 line
Jani Nikula, Intel
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
PATCH 10/11 drm/imx/tve: convert to struct drm_edid
In reply to Jani NikulaPrefer the struct drm_edid based functions for reading the EDID and updating the connector.
---
--- drivers/gpu/drm/imx/ipuv3/imx-tve.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
drivers/gpu/drm/imx/ipuv3/imx-tve.c +6 -8
@@ -201,18 +201,16 @@ static int tve_setup_vga(struct imx_tve *tve)static int imx_tve_connector_get_modes (struct drm_connector *connector) {struct imx_tve *tve =con_to_tve (connector); -struct edid *edid; -int ret =0 ; +const struct drm_edid *drm_edid; +int ret;if (!tve->ddc)return 0 ; - edid =drm_get_edid (connector, tve->ddc); -if (edid) { -drm_connector_update_edid_property (connector, edid); - ret =drm_add_edid_modes (connector, edid); -kfree (edid); - } + drm_edid =drm_edid_read_ddc (connector, tve->ddc); +drm_edid_connector_update (connector, drm_edid); + ret =drm_edid_connector_add_modes (connector); +drm_edid_free (drm_edid);return ret; }
Signature · 1 line
2.39.2
Re: [PATCH 10/11] drm/imx/tve: convert to struct drm_edid
Quoted 37 lines · Jani Nikula
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.Signed-off-by Jani Nikula <[email protected]>---
Cc Philipp Zabel <[email protected]>Cc Maarten Lankhorst <[email protected]>Cc Maxime Ripard <[email protected]>Cc Thomas Zimmermann <[email protected]>Cc Shawn Guo <[email protected]>Cc Sascha Hauer <[email protected]>Cc Pengutronix Kernel Team <[email protected]>Cc Fabio Estevam <[email protected]>--- drivers/gpu/drm/imx/ipuv3/imx-tve.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)drivers/gpu/drm/imx/ipuv3/imx-tve.c +2
-2@@ -201,18 +201,16 @@ static int tve_setup_vga(struct imx_tve *tve)static int imx_tve_connector_get_modes (struct drm_connector *connector) {struct imx_tve *tve =con_to_tve (connector); -struct edid *edid; -int ret =0 ; +const struct drm_edid *drm_edid; +int ret;if (!tve->ddc)return 0 ;
drivers/gpu/drm/imx/ipuv3/imx-tve.c static int tve_setup_vga(struct imx_tve *tve) · line 213
Quoted 10 lines
drivers/gpu/drm/imx/ipuv3/imx-tve.c +4
-6- edid =drm_get_edid (connector, tve->ddc); -if (edid) { -drm_connector_update_edid_property (connector, edid); - ret =drm_add_edid_modes (connector, edid); -kfree (edid); - } + drm_edid =drm_edid_read_ddc (connector, tve->ddc); +drm_edid_connector_update (connector, drm_edid); + ret =drm_edid_connector_add_modes (connector); +drm_edid_free (drm_edid);
Nit: if you change two last lines, you can drop ret= assignment and use return drm_edid_connector_add_modes(connector).
Maybe we shoud add cocci rule for such cases.
Quoted 6 lines
drivers/gpu/drm/imx/ipuv3/imx-tve.c +0
-0return ret; }Signature · 1 line
2.39.2
Signature · 2 lines
With best wishes Dmitry
Re: [PATCH 10/11] drm/imx/tve: convert to struct drm_edid
Quoted 51 lines · Dmitry Baryshkov
Quoted 37 lines · Jani Nikula
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.Signed-off-by Jani Nikula <[email protected]>---
Cc Philipp Zabel <[email protected]>Cc Maarten Lankhorst <[email protected]>Cc Maxime Ripard <[email protected]>Cc Thomas Zimmermann <[email protected]>Cc Shawn Guo <[email protected]>Cc Sascha Hauer <[email protected]>Cc Pengutronix Kernel Team <[email protected]>Cc Fabio Estevam <[email protected]>--- drivers/gpu/drm/imx/ipuv3/imx-tve.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)drivers/gpu/drm/imx/ipuv3/imx-tve.c +2
-2@@ -201,18 +201,16 @@ static int tve_setup_vga(struct imx_tve *tve)static int imx_tve_connector_get_modes (struct drm_connector *connector) {struct imx_tve *tve =con_to_tve (connector); -struct edid *edid; -int ret =0 ; +const struct drm_edid *drm_edid; +int ret;if (!tve->ddc)return 0 ;drivers/gpu/drm/imx/ipuv3/imx-tve.c static int tve_setup_vga(struct imx_tve *tve) · line 213
Quoted 10 lines
drivers/gpu/drm/imx/ipuv3/imx-tve.c +4
-6- edid =drm_get_edid (connector, tve->ddc); -if (edid) { -drm_connector_update_edid_property (connector, edid); - ret =drm_add_edid_modes (connector, edid); -kfree (edid); - } + drm_edid =drm_edid_read_ddc (connector, tve->ddc); +drm_edid_connector_update (connector, drm_edid); + ret =drm_edid_connector_add_modes (connector); +drm_edid_free (drm_edid);Reviewed-by Dmitry Baryshkov <[email protected]>
Quoted 4 lines
Nit: if you change two last lines, you can drop ret= assignment and use return
drm_edid_connector_add_modes(connector).Maybe we shoud add cocci rule for such cases.
I think there was a cocci rule like that, but a lot of people (including yours truly) preferred to keep the assignment, and the patches ended up in bikeshedding, so the cocci was removed.
My argument is that it's not uncommon to keep adding and removing stuff while the code evolves, and having to change the return statement is boring and makes the diff harder to follow. It's a bit like that extra comma at the end of initialization lists or enumeration definitions.
Others think the code should reflect current state and not prepare for scenarios that might never arrive.
Both are correct, so it's perfect for never ending bikeshedding. ;)
BR,
Jani.
Quoted 7 lines
Quoted 7 lines
drivers/gpu/drm/imx/ipuv3/imx-tve.c +0
-0return ret; }Signature · 1 line
2.39.2
Signature · 1 line
Jani Nikula, Intel
PATCH 11/11 drm/imx/ldb: convert to struct drm_edid
In reply to Jani NikulaPrefer the struct drm_edid based functions for reading the EDID and updating the connector.
---
--- drivers/gpu/drm/imx/ipuv3/imx-ldb.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
drivers/gpu/drm/imx/ipuv3/imx-ldb.c +12 -12
@@ -72,7 +72,7 @@ struct imx_ldb_channel {struct device_node *child;struct i2c_adapter *ddc;int chno; -void *edid; +const struct drm_edid *drm_edid;struct drm_display_mode mode;int mode_valid;u32 bus_format;
@@ -142,15 +142,15 @@ static int imx_ldb_connector_get_modes(struct drm_connector *connector)if (num_modes >0 )return num_modes; -if (!imx_ldb_ch->edid && imx_ldb_ch->ddc) - imx_ldb_ch->edid =drm_get_edid (connector, imx_ldb_ch->ddc); - -if (imx_ldb_ch->edid) { -drm_connector_update_edid_property (connector, - imx_ldb_ch->edid); - num_modes =drm_add_edid_modes (connector, imx_ldb_ch->edid); +if (!imx_ldb_ch->drm_edid && imx_ldb_ch->ddc) { + imx_ldb_ch->drm_edid =drm_edid_read_ddc (connector, + imx_ldb_ch->ddc); +drm_edid_connector_update (connector, imx_ldb_ch->drm_edid); } +if (imx_ldb_ch->drm_edid) + num_modes =drm_edid_connector_add_modes (connector); +if (imx_ldb_ch->mode_valid) {struct drm_display_mode *mode;
@@ -553,7 +553,6 @@ static int imx_ldb_panel_ddc(struct device *dev, struct imx_ldb_channel *channel, struct device_node *child) { struct device_node *ddc_node; -const u8 *edidp;int ret; ddc_node =of_parse_phandle (child,"ddc-i2c-bus" ,0 );
@@ -567,6 +566,7 @@ static int imx_ldb_panel_ddc(struct device *dev, }if (!channel->ddc) { +const void *edidp;int edid_len;/* if no DDC available, fallback to hardcoded EDID */
@@ -574,8 +574,8 @@ static int imx_ldb_panel_ddc(struct device *dev, edidp =of_get_property (child,"edid" , &edid_len);if (edidp) { - channel->edid =kmemdup (edidp, edid_len,GFP_KERNEL ); -if (!channel->edid) + channel->drm_edid =drm_edid_alloc (edidp, edid_len); +if (!channel->drm_edid)return -ENOMEM ; }else if (!channel->panel) {/* fallback to display-timings node */
@@ -744,7 +744,7 @@ static void imx_ldb_remove(struct platform_device *pdev)for (i =0 ; i <2 ; i++) {struct imx_ldb_channel *channel = &imx_ldb->channel[i]; -kfree (channel->edid); +drm_edid_free (channel->drm_edid);i2c_put_adapter (channel->ddc); }
Signature · 1 line
2.39.2
Re: [PATCH 11/11] drm/imx/ldb: convert to struct drm_edid
Quoted 17 lines · Jani Nikula
Prefer the struct
drm_edidbased functions for reading the EDID and updating the connector.Signed-off-by Jani Nikula <[email protected]>---
Cc Philipp Zabel <[email protected]>Cc Maarten Lankhorst <[email protected]>Cc Maxime Ripard <[email protected]>Cc Thomas Zimmermann <[email protected]>Cc Shawn Guo <[email protected]>Cc Sascha Hauer <[email protected]>Cc Pengutronix Kernel Team <[email protected]>Cc Fabio Estevam <[email protected]>
Quoted 3 lines
--- drivers/gpu/drm/imx/ipuv3/imx-ldb.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
Note: https://lore.kernel.org/dri-devel/[email protected]/
Quoted 75 lines
drivers/gpu/drm/imx/ipuv3/imx-ldb.c +12
-12@@ -72,7 +72,7 @@ struct imx_ldb_channel {struct device_node *child;struct i2c_adapter *ddc;int chno; -void *edid; +const struct drm_edid *drm_edid;struct drm_display_mode mode;int mode_valid;u32 bus_format;@@ -142,15 +142,15 @@ static int imx_ldb_connector_get_modes(struct drm_connector *connector)if (num_modes >0 )return num_modes; -if (!imx_ldb_ch->edid && imx_ldb_ch->ddc) - imx_ldb_ch->edid =drm_get_edid (connector, imx_ldb_ch->ddc); - -if (imx_ldb_ch->edid) { -drm_connector_update_edid_property (connector, - imx_ldb_ch->edid); - num_modes =drm_add_edid_modes (connector, imx_ldb_ch->edid); +if (!imx_ldb_ch->drm_edid && imx_ldb_ch->ddc) { + imx_ldb_ch->drm_edid =drm_edid_read_ddc (connector, + imx_ldb_ch->ddc); +drm_edid_connector_update (connector, imx_ldb_ch->drm_edid); } +if (imx_ldb_ch->drm_edid) + num_modes =drm_edid_connector_add_modes (connector); +if (imx_ldb_ch->mode_valid) {struct drm_display_mode *mode;@@ -553,7 +553,6 @@ static int imx_ldb_panel_ddc(struct device *dev, struct imx_ldb_channel *channel, struct device_node *child) { struct device_node *ddc_node; -const u8 *edidp;int ret; ddc_node =of_parse_phandle (child,"ddc-i2c-bus" ,0 );@@ -567,6 +566,7 @@ static int imx_ldb_panel_ddc(struct device *dev, }if (!channel->ddc) { +const void *edidp;int edid_len;/* if no DDC available, fallback to hardcoded EDID */ @@ -574,8 +574,8 @@ static int imx_ldb_panel_ddc(struct device *dev, edidp =of_get_property (child,"edid" , &edid_len);if (edidp) { - channel->edid =kmemdup (edidp, edid_len,GFP_KERNEL ); -if (!channel->edid) + channel->drm_edid =drm_edid_alloc (edidp, edid_len); +if (!channel->drm_edid)return -ENOMEM ; }else if (!channel->panel) {/* fallback to display-timings node */ @@ -744,7 +744,7 @@ static void imx_ldb_remove(struct platform_device *pdev)for (i =0 ; i <2 ; i++) {struct imx_ldb_channel *channel = &imx_ldb->channel[i]; -kfree (channel->edid); +drm_edid_free (channel->drm_edid);i2c_put_adapter (channel->ddc); }Signature · 1 line
2.39.2
Signature · 2 lines
With best wishes Dmitry
Dmitry Baryshkov
AppliedQuoted 17 lines · Jani Nikula
Convert more drivers to struct
drm_edid.Compile tested only.
Jani Nikula (11):
drm/rockchip: cdn-dp: get rid ofdrm_edid_raw()drm/sti/sti_hdmi: convert to structdrm_ediddrm/bridge:analogix_dp: convert to structdrm_ediddrm/exynos: hdmi: convert to structdrm_ediddrm/hisilicon/hibmc: convert to structdrm_ediddrm/loongson/7a1000: convert to structdrm_ediddrm/loongson/7a2000: convert to structdrm_ediddrm/msm/dp: switch to structdrm_ediddrm/tegra: convert to structdrm_ediddrm/imx/tve: convert to structdrm_ediddrm/imx/ldb: convert to structdrm_edid
Quoted 1 line
[...]
Applied, thanks!
[08/11] drm/msm/dp: switch to struct drm_edid
https://gitlab.freedesktop.org/lumag/msm/-/commit/b1510bee17f5
Best regards,
Signature · 1 line
Dmitry Baryshkov <[email protected]>