| Message ID | 20240818-radeon-drm_edid-v1-1-4b7fdd19132e@weissschuh.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/radeon: Switch radeon_connector to struct drm_edid | expand |
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,
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
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);
"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(-) --- base-commit: 19cff16559a4f2d763faf4f8392bf86d3a21b93c change-id: 20240818-radeon-drm_edid-9f0cec36e227 Best regards,