[PATCH] drm/radeon: Switch radeon_connector to struct drm_edid

Jani Nikula jani.nikula at linux.intel.com
Mon Aug 19 08:51:45 UTC 2024


On Sun, 18 Aug 2024, Thomas Weißschuh <linux at weissschuh.net> wrote:
> "struct drm_edid" is the safe and recommended alternative to "struct edid".
>
> Rename the member to make sure that no usage sites are missed,
> as "struct drm_edid" has some restrictions, for example it can not be
> used with kfree().
>
> Signed-off-by: Thomas Weißschuh <linux at weissschuh.net>
> ---
> This is only compile-tested.
> ---
>  drivers/gpu/drm/radeon/radeon_audio.c      |  4 +--
>  drivers/gpu/drm/radeon/radeon_combios.c    |  4 +--
>  drivers/gpu/drm/radeon/radeon_connectors.c | 54 +++++++++++++++---------------
>  drivers/gpu/drm/radeon/radeon_mode.h       |  5 ++-
>  4 files changed, 33 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c
> index 47aa06a9a942..59d0e47c94d0 100644
> --- a/drivers/gpu/drm/radeon/radeon_audio.c
> +++ b/drivers/gpu/drm/radeon/radeon_audio.c
> @@ -311,7 +311,7 @@ static void radeon_audio_write_sad_regs(struct drm_encoder *encoder)
>  	if (!connector)
>  		return;
>  
> -	sad_count = drm_edid_to_sad(radeon_connector->edid, &sads);
> +	sad_count = drm_edid_to_sad(drm_edid_raw(radeon_connector->drm_edid), &sads);

The drm_edid_raw() usages could use a FIXME comment; eventually we want
all of them gone too.

>  	if (sad_count < 0)
>  		DRM_ERROR("Couldn't read SADs: %d\n", sad_count);
>  	if (sad_count <= 0)
> @@ -335,7 +335,7 @@ static void radeon_audio_write_speaker_allocation(struct drm_encoder *encoder)
>  	if (!connector)
>  		return;
>  
> -	sad_count = drm_edid_to_speaker_allocation(radeon_connector->edid, &sadb);
> +	sad_count = drm_edid_to_speaker_allocation(drm_edid_raw(radeon_connector->drm_edid), &sadb);
>  	if (sad_count < 0) {
>  		DRM_DEBUG("Couldn't read Speaker Allocation Data Block: %d\n",
>  			  sad_count);
> diff --git a/drivers/gpu/drm/radeon/radeon_combios.c b/drivers/gpu/drm/radeon/radeon_combios.c
> index df8d7f56b028..f4947acd0419 100644
> --- a/drivers/gpu/drm/radeon/radeon_combios.c
> +++ b/drivers/gpu/drm/radeon/radeon_combios.c
> @@ -390,10 +390,10 @@ bool radeon_combios_check_hardcoded_edid(struct radeon_device *rdev)
>  }
>  
>  /* this is used for atom LCDs as well */
> -struct edid *
> +const struct drm_edid *
>  radeon_bios_get_hardcoded_edid(struct radeon_device *rdev)
>  {
> -	return drm_edid_duplicate(drm_edid_raw(rdev->mode_info.bios_hardcoded_edid));
> +	return drm_edid_dup(rdev->mode_info.bios_hardcoded_edid);
>  }
>  
>  static struct radeon_i2c_bus_rec combios_setup_i2c_bus(struct radeon_device *rdev,
> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
> index 528a8f3677c2..87a78c8e09c0 100644
> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> @@ -261,7 +261,7 @@ static void radeon_connector_get_edid(struct drm_connector *connector)
>  	struct radeon_device *rdev = dev->dev_private;
>  	struct radeon_connector *radeon_connector = to_radeon_connector(connector);
>  
> -	if (radeon_connector->edid)
> +	if (radeon_connector->drm_edid)
>  		return;
>  
>  	/* on hw with routers, select right port */
> @@ -271,8 +271,8 @@ static void radeon_connector_get_edid(struct drm_connector *connector)
>  	if ((radeon_connector_encoder_get_dp_bridge_encoder_id(connector) !=
>  	     ENCODER_OBJECT_ID_NONE) &&
>  	    radeon_connector->ddc_bus->has_aux) {
> -		radeon_connector->edid = drm_get_edid(connector,
> -						      &radeon_connector->ddc_bus->aux.ddc);
> +		radeon_connector->drm_edid = drm_edid_read_ddc(connector,
> +							       &radeon_connector->ddc_bus->aux.ddc);
>  	} else if ((connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) ||
>  		   (connector->connector_type == DRM_MODE_CONNECTOR_eDP)) {
>  		struct radeon_connector_atom_dig *dig = radeon_connector->con_priv;
> @@ -280,22 +280,22 @@ static void radeon_connector_get_edid(struct drm_connector *connector)
>  		if ((dig->dp_sink_type == CONNECTOR_OBJECT_ID_DISPLAYPORT ||
>  		     dig->dp_sink_type == CONNECTOR_OBJECT_ID_eDP) &&
>  		    radeon_connector->ddc_bus->has_aux)
> -			radeon_connector->edid = drm_get_edid(&radeon_connector->base,
> -							      &radeon_connector->ddc_bus->aux.ddc);
> +			radeon_connector->drm_edid = drm_edid_read_ddc(&radeon_connector->base,
> +								       &radeon_connector->ddc_bus->aux.ddc);
>  		else if (radeon_connector->ddc_bus)
> -			radeon_connector->edid = drm_get_edid(&radeon_connector->base,
> -							      &radeon_connector->ddc_bus->adapter);
> +			radeon_connector->drm_edid = drm_edid_read_ddc(&radeon_connector->base,
> +								       &radeon_connector->ddc_bus->adapter);
>  	} else if (vga_switcheroo_handler_flags() & VGA_SWITCHEROO_CAN_SWITCH_DDC &&
>  		   connector->connector_type == DRM_MODE_CONNECTOR_LVDS &&
>  		   radeon_connector->ddc_bus) {
> -		radeon_connector->edid = drm_get_edid_switcheroo(&radeon_connector->base,
> -								 &radeon_connector->ddc_bus->adapter);
> +		radeon_connector->drm_edid = drm_edid_read_ddc(&radeon_connector->base,
> +							       &radeon_connector->ddc_bus->adapter);

drm_edid_read_switcheroo()!


>  	} else if (radeon_connector->ddc_bus) {
> -		radeon_connector->edid = drm_get_edid(&radeon_connector->base,
> -						      &radeon_connector->ddc_bus->adapter);
> +		radeon_connector->drm_edid = drm_edid_read_ddc(&radeon_connector->base,
> +							       &radeon_connector->ddc_bus->adapter);
>  	}
>  
> -	if (!radeon_connector->edid) {
> +	if (!radeon_connector->drm_edid) {
>  		/* don't fetch the edid from the vbios if ddc fails and runpm is
>  		 * enabled so we report disconnected.
>  		 */
> @@ -306,20 +306,22 @@ static void radeon_connector_get_edid(struct drm_connector *connector)
>  			/* some laptops provide a hardcoded edid in rom for LCDs */
>  			if (((connector->connector_type == DRM_MODE_CONNECTOR_LVDS) ||
>  			     (connector->connector_type == DRM_MODE_CONNECTOR_eDP)))
> -				radeon_connector->edid = radeon_bios_get_hardcoded_edid(rdev);
> +				radeon_connector->drm_edid = radeon_bios_get_hardcoded_edid(rdev);
>  		} else {
>  			/* some servers provide a hardcoded edid in rom for KVMs */
> -			radeon_connector->edid = radeon_bios_get_hardcoded_edid(rdev);
> +			radeon_connector->drm_edid = radeon_bios_get_hardcoded_edid(rdev);
>  		}
>  	}
> +
> +	drm_edid_connector_update(&radeon_connector->base, radeon_connector->drm_edid);
>  }
>  
>  static void radeon_connector_free_edid(struct drm_connector *connector)
>  {
>  	struct radeon_connector *radeon_connector = to_radeon_connector(connector);
>  
> -	kfree(radeon_connector->edid);
> -	radeon_connector->edid = NULL;
> +	drm_edid_free(radeon_connector->drm_edid);
> +	radeon_connector->drm_edid = NULL;
>  }
>  
>  static int radeon_ddc_get_modes(struct drm_connector *connector)
> @@ -327,12 +329,12 @@ static int radeon_ddc_get_modes(struct drm_connector *connector)
>  	struct radeon_connector *radeon_connector = to_radeon_connector(connector);
>  	int ret;
>  
> -	if (radeon_connector->edid) {
> -		drm_connector_update_edid_property(connector, radeon_connector->edid);
> -		ret = drm_add_edid_modes(connector, radeon_connector->edid);
> +	if (radeon_connector->drm_edid) {
> +		drm_edid_connector_update(connector, radeon_connector->drm_edid);
> +		ret = drm_edid_connector_add_modes(connector);
>  		return ret;
>  	}
> -	drm_connector_update_edid_property(connector, NULL);
> +	drm_edid_connector_update(connector, NULL);

The whole thing above can be simplified, as the functions handle NULL
parameters just fine.

>  	return 0;
>  }
>  
> @@ -869,7 +871,7 @@ radeon_lvds_detect(struct drm_connector *connector, bool force)
>  
>  	/* check for edid as well */
>  	radeon_connector_get_edid(connector);
> -	if (radeon_connector->edid)
> +	if (radeon_connector->drm_edid)
>  		ret = connector_status_connected;
>  	/* check acpi lid status ??? */
>  
> @@ -1012,13 +1014,12 @@ radeon_vga_detect(struct drm_connector *connector, bool force)
>  		radeon_connector_free_edid(connector);
>  		radeon_connector_get_edid(connector);
>  
> -		if (!radeon_connector->edid) {
> +		if (!radeon_connector->drm_edid) {
>  			DRM_ERROR("%s: probed a monitor but no|invalid EDID\n",
>  					connector->name);
>  			ret = connector_status_connected;
>  		} else {
> -			radeon_connector->use_digital =
> -				!!(radeon_connector->edid->input & DRM_EDID_INPUT_DIGITAL);
> +			radeon_connector->use_digital = drm_edid_is_digital(radeon_connector->drm_edid);
>  
>  			/* some oems have boards with separate digital and analog connectors
>  			 * with a shared ddc line (often vga + hdmi)
> @@ -1270,7 +1271,7 @@ radeon_dvi_detect(struct drm_connector *connector, bool force)
>  		radeon_connector_free_edid(connector);
>  		radeon_connector_get_edid(connector);
>  
> -		if (!radeon_connector->edid) {
> +		if (!radeon_connector->drm_edid) {
>  			DRM_ERROR("%s: probed a monitor but no|invalid EDID\n",
>  					connector->name);
>  			/* rs690 seems to have a problem with connectors not existing and always
> @@ -1286,8 +1287,7 @@ radeon_dvi_detect(struct drm_connector *connector, bool force)
>  				broken_edid = true; /* defer use_digital to later */
>  			}
>  		} else {
> -			radeon_connector->use_digital =
> -				!!(radeon_connector->edid->input & DRM_EDID_INPUT_DIGITAL);
> +			radeon_connector->use_digital = drm_edid_is_digital(radeon_connector->drm_edid);
>  
>  			/* some oems have boards with separate digital and analog connectors
>  			 * with a shared ddc line (often vga + hdmi)
> diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
> index 421c83fc70dc..ae1d91cd93ec 100644
> --- a/drivers/gpu/drm/radeon/radeon_mode.h
> +++ b/drivers/gpu/drm/radeon/radeon_mode.h
> @@ -38,7 +38,6 @@
>  #include <linux/i2c.h>
>  #include <linux/i2c-algo-bit.h>
>  
> -struct edid;
>  struct drm_edid;
>  struct radeon_bo;
>  struct radeon_device;
> @@ -521,7 +520,7 @@ struct radeon_connector {
>  	bool use_digital;
>  	/* we need to mind the EDID between detect
>  	   and get modes due to analog/digital/tvencoder */
> -	struct edid *edid;
> +	const struct drm_edid *drm_edid;
>  	void *con_priv;
>  	bool dac_load_detect;
>  	bool detected_by_load; /* if the connection status was determined by load */
> @@ -843,7 +842,7 @@ radeon_get_crtc_scanout_position(struct drm_crtc *crtc, bool in_vblank_irq,
>  				 const struct drm_display_mode *mode);
>  
>  extern bool radeon_combios_check_hardcoded_edid(struct radeon_device *rdev);
> -extern struct edid *
> +extern const struct drm_edid *
>  radeon_bios_get_hardcoded_edid(struct radeon_device *rdev);
>  extern bool radeon_atom_get_clock_info(struct drm_device *dev);
>  extern bool radeon_combios_get_clock_info(struct drm_device *dev);
>
> ---
> base-commit: 19cff16559a4f2d763faf4f8392bf86d3a21b93c
> change-id: 20240818-radeon-drm_edid-9f0cec36e227
>
> Best regards,

-- 
Jani Nikula, Intel


More information about the dri-devel mailing list