diff mbox series

[V3] drm/amd/display: Remove unwanted drm edid references

Message ID 20230918130455.2868485-1-alex.hung@amd.com (mailing list archive)
State New, archived
Headers show
Series [V3] drm/amd/display: Remove unwanted drm edid references | expand

Commit Message

Alex Hung Sept. 18, 2023, 1:04 p.m. UTC
[WHY]
edid_override and drm_edid_override_connector_update, according to drm
documentation, should not be referred outside drm_edid.

[HOW]
Remove and replace them accordingly. This can tested by IGT's
kms_hdmi_inject test.

Signed-off-by: Alex Hung <alex.hung@amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 42 ++++++++++---------
 1 file changed, 23 insertions(+), 19 deletions(-)

Comments

Jani Nikula Sept. 19, 2023, 9:53 a.m. UTC | #1
On Mon, 18 Sep 2023, Alex Hung <alex.hung@amd.com> wrote:
> [WHY]
> edid_override and drm_edid_override_connector_update, according to drm
> documentation, should not be referred outside drm_edid.
>
> [HOW]
> Remove and replace them accordingly. This can tested by IGT's
> kms_hdmi_inject test.
>
> Signed-off-by: Alex Hung <alex.hung@amd.com>

FWIW, Acked-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 42 ++++++++++---------
>  1 file changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 5efebc06296b..3968dd9cef7f 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -6444,15 +6444,23 @@ amdgpu_dm_connector_late_register(struct drm_connector *connector)
>  static void amdgpu_dm_connector_funcs_force(struct drm_connector *connector)
>  {
>  	struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector);
> +	struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(connector);
>  	struct dc_link *dc_link = aconnector->dc_link;
>  	struct dc_sink *dc_em_sink = aconnector->dc_em_sink;
>  	struct edid *edid;
>  
> -	if (!connector->edid_override)
> +	/*
> +	 * Note: drm_get_edid gets edid in the following order:
> +	 * 1) override EDID if set via edid_override debugfs,
> +	 * 2) firmware EDID if set via edid_firmware module parameter
> +	 * 3) regular DDC read.
> +	 */
> +	edid = drm_get_edid(connector, &amdgpu_connector->ddc_bus->aux.ddc);
> +	if (!edid) {
> +		DRM_ERROR("No EDID found on connector: %s.\n", connector->name);
>  		return;
> +	}
>  
> -	drm_edid_override_connector_update(&aconnector->base);
> -	edid = aconnector->base.edid_blob_ptr->data;
>  	aconnector->edid = edid;
>  
>  	/* Update emulated (virtual) sink's EDID */
> @@ -6487,30 +6495,26 @@ static int get_modes(struct drm_connector *connector)
>  
>  static void create_eml_sink(struct amdgpu_dm_connector *aconnector)
>  {
> +	struct drm_connector *connector = &aconnector->base;
> +	struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(&aconnector->base);
>  	struct dc_sink_init_data init_params = {
>  			.link = aconnector->dc_link,
>  			.sink_signal = SIGNAL_TYPE_VIRTUAL
>  	};
>  	struct edid *edid;
>  
> -	if (!aconnector->base.edid_blob_ptr) {
> -		/* if connector->edid_override valid, pass
> -		 * it to edid_override to edid_blob_ptr
> -		 */
> -
> -		drm_edid_override_connector_update(&aconnector->base);
> -
> -		if (!aconnector->base.edid_blob_ptr) {
> -			DRM_ERROR("No EDID firmware found on connector: %s ,forcing to OFF!\n",
> -					aconnector->base.name);
> -
> -			aconnector->base.force = DRM_FORCE_OFF;
> -			return;
> -		}
> +	/*
> +	 * Note: drm_get_edid gets edid in the following order:
> +	 * 1) override EDID if set via edid_override debugfs,
> +	 * 2) firmware EDID if set via edid_firmware module parameter
> +	 * 3) regular DDC read.
> +	 */
> +	edid = drm_get_edid(connector, &amdgpu_connector->ddc_bus->aux.ddc);
> +	if (!edid) {
> +		DRM_ERROR("No EDID found on connector: %s.\n", connector->name);
> +		return;
>  	}
>  
> -	edid = (struct edid *) aconnector->base.edid_blob_ptr->data;
> -
>  	aconnector->edid = edid;
>  
>  	aconnector->dc_em_sink = dc_link_add_remote_sink(
Harry Wentland Sept. 22, 2023, 8:11 p.m. UTC | #2
On 2023-09-18 09:04, Alex Hung wrote:
> [WHY]
> edid_override and drm_edid_override_connector_update, according to drm
> documentation, should not be referred outside drm_edid.
> 
> [HOW]
> Remove and replace them accordingly. This can tested by IGT's
> kms_hdmi_inject test.
> 
> Signed-off-by: Alex Hung <alex.hung@amd.com>

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 42 ++++++++++---------
>   1 file changed, 23 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 5efebc06296b..3968dd9cef7f 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -6444,15 +6444,23 @@ amdgpu_dm_connector_late_register(struct drm_connector *connector)
>   static void amdgpu_dm_connector_funcs_force(struct drm_connector *connector)
>   {
>   	struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector);
> +	struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(connector);
>   	struct dc_link *dc_link = aconnector->dc_link;
>   	struct dc_sink *dc_em_sink = aconnector->dc_em_sink;
>   	struct edid *edid;
>   
> -	if (!connector->edid_override)
> +	/*
> +	 * Note: drm_get_edid gets edid in the following order:
> +	 * 1) override EDID if set via edid_override debugfs,
> +	 * 2) firmware EDID if set via edid_firmware module parameter
> +	 * 3) regular DDC read.
> +	 */
> +	edid = drm_get_edid(connector, &amdgpu_connector->ddc_bus->aux.ddc);
> +	if (!edid) {
> +		DRM_ERROR("No EDID found on connector: %s.\n", connector->name);
>   		return;
> +	}
>   
> -	drm_edid_override_connector_update(&aconnector->base);
> -	edid = aconnector->base.edid_blob_ptr->data;
>   	aconnector->edid = edid;
>   
>   	/* Update emulated (virtual) sink's EDID */
> @@ -6487,30 +6495,26 @@ static int get_modes(struct drm_connector *connector)
>   
>   static void create_eml_sink(struct amdgpu_dm_connector *aconnector)
>   {
> +	struct drm_connector *connector = &aconnector->base;
> +	struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(&aconnector->base);
>   	struct dc_sink_init_data init_params = {
>   			.link = aconnector->dc_link,
>   			.sink_signal = SIGNAL_TYPE_VIRTUAL
>   	};
>   	struct edid *edid;
>   
> -	if (!aconnector->base.edid_blob_ptr) {
> -		/* if connector->edid_override valid, pass
> -		 * it to edid_override to edid_blob_ptr
> -		 */
> -
> -		drm_edid_override_connector_update(&aconnector->base);
> -
> -		if (!aconnector->base.edid_blob_ptr) {
> -			DRM_ERROR("No EDID firmware found on connector: %s ,forcing to OFF!\n",
> -					aconnector->base.name);
> -
> -			aconnector->base.force = DRM_FORCE_OFF;
> -			return;
> -		}
> +	/*
> +	 * Note: drm_get_edid gets edid in the following order:
> +	 * 1) override EDID if set via edid_override debugfs,
> +	 * 2) firmware EDID if set via edid_firmware module parameter
> +	 * 3) regular DDC read.
> +	 */
> +	edid = drm_get_edid(connector, &amdgpu_connector->ddc_bus->aux.ddc);
> +	if (!edid) {
> +		DRM_ERROR("No EDID found on connector: %s.\n", connector->name);
> +		return;
>   	}
>   
> -	edid = (struct edid *) aconnector->base.edid_blob_ptr->data;
> -
>   	aconnector->edid = edid;
>   
>   	aconnector->dc_em_sink = dc_link_add_remote_sink(
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 5efebc06296b..3968dd9cef7f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6444,15 +6444,23 @@  amdgpu_dm_connector_late_register(struct drm_connector *connector)
 static void amdgpu_dm_connector_funcs_force(struct drm_connector *connector)
 {
 	struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector);
+	struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(connector);
 	struct dc_link *dc_link = aconnector->dc_link;
 	struct dc_sink *dc_em_sink = aconnector->dc_em_sink;
 	struct edid *edid;
 
-	if (!connector->edid_override)
+	/*
+	 * Note: drm_get_edid gets edid in the following order:
+	 * 1) override EDID if set via edid_override debugfs,
+	 * 2) firmware EDID if set via edid_firmware module parameter
+	 * 3) regular DDC read.
+	 */
+	edid = drm_get_edid(connector, &amdgpu_connector->ddc_bus->aux.ddc);
+	if (!edid) {
+		DRM_ERROR("No EDID found on connector: %s.\n", connector->name);
 		return;
+	}
 
-	drm_edid_override_connector_update(&aconnector->base);
-	edid = aconnector->base.edid_blob_ptr->data;
 	aconnector->edid = edid;
 
 	/* Update emulated (virtual) sink's EDID */
@@ -6487,30 +6495,26 @@  static int get_modes(struct drm_connector *connector)
 
 static void create_eml_sink(struct amdgpu_dm_connector *aconnector)
 {
+	struct drm_connector *connector = &aconnector->base;
+	struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(&aconnector->base);
 	struct dc_sink_init_data init_params = {
 			.link = aconnector->dc_link,
 			.sink_signal = SIGNAL_TYPE_VIRTUAL
 	};
 	struct edid *edid;
 
-	if (!aconnector->base.edid_blob_ptr) {
-		/* if connector->edid_override valid, pass
-		 * it to edid_override to edid_blob_ptr
-		 */
-
-		drm_edid_override_connector_update(&aconnector->base);
-
-		if (!aconnector->base.edid_blob_ptr) {
-			DRM_ERROR("No EDID firmware found on connector: %s ,forcing to OFF!\n",
-					aconnector->base.name);
-
-			aconnector->base.force = DRM_FORCE_OFF;
-			return;
-		}
+	/*
+	 * Note: drm_get_edid gets edid in the following order:
+	 * 1) override EDID if set via edid_override debugfs,
+	 * 2) firmware EDID if set via edid_firmware module parameter
+	 * 3) regular DDC read.
+	 */
+	edid = drm_get_edid(connector, &amdgpu_connector->ddc_bus->aux.ddc);
+	if (!edid) {
+		DRM_ERROR("No EDID found on connector: %s.\n", connector->name);
+		return;
 	}
 
-	edid = (struct edid *) aconnector->base.edid_blob_ptr->data;
-
 	aconnector->edid = edid;
 
 	aconnector->dc_em_sink = dc_link_add_remote_sink(