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(-)
"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@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);
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);
} 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);
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,
--
Thomas Weißschuh <linux@weissschuh.net>
On Sun, 18 Aug 2024, Thomas Weißschuh <linux@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@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
On 2024-08-19 11:51:45+0000, Jani Nikula wrote:
> On Sun, 18 Aug 2024, Thomas Weißschuh <linux@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@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.
amdgpu and radeon are the last users of drm_edid_to_sad() and
drm_edid_to_speaker_allocation() which do not have access to a drm_edid.
After that we can fix the to function to directly take the drm_edid.
(I have a patch for it)
Personally I dislike to have so many FIXMEs in the code.
>
> > 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()!
Ack.
>
>
> > } 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.
Ack.
>
> > 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
© 2016 - 2026 Red Hat, Inc.