| Message ID | c31c3afa883a3321345608c480c26161b638a83e.1724348429.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: conversions to struct drm_edid | expand |
On Thu, Aug 22, 2024 at 08:42:52PM +0300, Jani Nikula wrote: > Prefer the struct drm_edid based functions for allocating the EDID and > updating the connector. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > --- > > Cc: Hans de Goede <hdegoede@redhat.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: dri-devel@lists.freedesktop.org > --- > drivers/gpu/drm/tiny/gm12u320.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c > index e0defb1d134f..0bd7707c053e 100644 > --- a/drivers/gpu/drm/tiny/gm12u320.c > +++ b/drivers/gpu/drm/tiny/gm12u320.c > @@ -464,7 +464,7 @@ static int gm12u320_set_ecomode(struct gm12u320_device *gm12u320) > * Note this assumes this driver is only ever used with the Acer C120, if we > * add support for other devices the vendor and model should be parameterized. > */ > -static struct edid gm12u320_edid = { > +static const struct edid gm12u320_edid = { > .header = { 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00 }, > .mfg_id = { 0x04, 0x72 }, /* "ACR" */ > .prod_code = { 0x20, 0xc1 }, /* C120h */ > @@ -523,8 +523,15 @@ static struct edid gm12u320_edid = { > > static int gm12u320_conn_get_modes(struct drm_connector *connector) > { > - drm_connector_update_edid_property(connector, &gm12u320_edid); > - return drm_add_edid_modes(connector, &gm12u320_edid); > + const struct drm_edid *drm_edid; > + int count; > + > + drm_edid = drm_edid_alloc(&gm12u320_edid, sizeof(gm12u320_edid)); > + drm_edid_connector_update(connector, drm_edid); > + count = drm_edid_connector_add_modes(connector); > + drm_edid_free(drm_edid); > + > + return count; > } > > static const struct drm_connector_helper_funcs gm12u320_conn_helper_funcs = { > -- > 2.39.2 >
Hi Am 22.08.24 um 19:42 schrieb Jani Nikula: > Prefer the struct drm_edid based functions for allocating the EDID and > updating the connector. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > --- > > Cc: Hans de Goede <hdegoede@redhat.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: dri-devel@lists.freedesktop.org > --- > drivers/gpu/drm/tiny/gm12u320.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c > index e0defb1d134f..0bd7707c053e 100644 > --- a/drivers/gpu/drm/tiny/gm12u320.c > +++ b/drivers/gpu/drm/tiny/gm12u320.c > @@ -464,7 +464,7 @@ static int gm12u320_set_ecomode(struct gm12u320_device *gm12u320) > * Note this assumes this driver is only ever used with the Acer C120, if we > * add support for other devices the vendor and model should be parameterized. > */ > -static struct edid gm12u320_edid = { > +static const struct edid gm12u320_edid = { > .header = { 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00 }, > .mfg_id = { 0x04, 0x72 }, /* "ACR" */ > .prod_code = { 0x20, 0xc1 }, /* C120h */ > @@ -523,8 +523,15 @@ static struct edid gm12u320_edid = { > > static int gm12u320_conn_get_modes(struct drm_connector *connector) > { > - drm_connector_update_edid_property(connector, &gm12u320_edid); > - return drm_add_edid_modes(connector, &gm12u320_edid); > + const struct drm_edid *drm_edid; > + int count; > + > + drm_edid = drm_edid_alloc(&gm12u320_edid, sizeof(gm12u320_edid)); Wouldn't it be better to use drm_edid_read_custom() to get the connector's edid override, if any? Best regards Thomas > + drm_edid_connector_update(connector, drm_edid); > + count = drm_edid_connector_add_modes(connector); > + drm_edid_free(drm_edid); > + > + return count; > } > > static const struct drm_connector_helper_funcs gm12u320_conn_helper_funcs = {
On Wed, 28 Aug 2024, Thomas Zimmermann <tzimmermann@suse.de> wrote: > Hi > > Am 22.08.24 um 19:42 schrieb Jani Nikula: >> Prefer the struct drm_edid based functions for allocating the EDID and >> updating the connector. >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> >> --- >> >> Cc: Hans de Goede <hdegoede@redhat.com> >> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> Cc: Maxime Ripard <mripard@kernel.org> >> Cc: Thomas Zimmermann <tzimmermann@suse.de> >> Cc: dri-devel@lists.freedesktop.org >> --- >> drivers/gpu/drm/tiny/gm12u320.c | 13 ++++++++++--- >> 1 file changed, 10 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c >> index e0defb1d134f..0bd7707c053e 100644 >> --- a/drivers/gpu/drm/tiny/gm12u320.c >> +++ b/drivers/gpu/drm/tiny/gm12u320.c >> @@ -464,7 +464,7 @@ static int gm12u320_set_ecomode(struct gm12u320_device *gm12u320) >> * Note this assumes this driver is only ever used with the Acer C120, if we >> * add support for other devices the vendor and model should be parameterized. >> */ >> -static struct edid gm12u320_edid = { >> +static const struct edid gm12u320_edid = { >> .header = { 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00 }, >> .mfg_id = { 0x04, 0x72 }, /* "ACR" */ >> .prod_code = { 0x20, 0xc1 }, /* C120h */ >> @@ -523,8 +523,15 @@ static struct edid gm12u320_edid = { >> >> static int gm12u320_conn_get_modes(struct drm_connector *connector) >> { >> - drm_connector_update_edid_property(connector, &gm12u320_edid); >> - return drm_add_edid_modes(connector, &gm12u320_edid); >> + const struct drm_edid *drm_edid; >> + int count; >> + >> + drm_edid = drm_edid_alloc(&gm12u320_edid, sizeof(gm12u320_edid)); > > Wouldn't it be better to use drm_edid_read_custom() to get the > connector's edid override, if any? It would, no doubt. But it's more work, and the conversion is more error prone. I don't have the devices to test any of this, and I don't see many people jumping in to help with that either. So I'm opting for the minimum but positive incremental improvements with the drm_edid conversions. BR, Jani. > > Best regards > Thomas > >> + drm_edid_connector_update(connector, drm_edid); >> + count = drm_edid_connector_add_modes(connector); >> + drm_edid_free(drm_edid); >> + >> + return count; >> } >> >> static const struct drm_connector_helper_funcs gm12u320_conn_helper_funcs = {
diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c index e0defb1d134f..0bd7707c053e 100644 --- a/drivers/gpu/drm/tiny/gm12u320.c +++ b/drivers/gpu/drm/tiny/gm12u320.c @@ -464,7 +464,7 @@ static int gm12u320_set_ecomode(struct gm12u320_device *gm12u320) * Note this assumes this driver is only ever used with the Acer C120, if we * add support for other devices the vendor and model should be parameterized. */ -static struct edid gm12u320_edid = { +static const struct edid gm12u320_edid = { .header = { 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00 }, .mfg_id = { 0x04, 0x72 }, /* "ACR" */ .prod_code = { 0x20, 0xc1 }, /* C120h */ @@ -523,8 +523,15 @@ static struct edid gm12u320_edid = { static int gm12u320_conn_get_modes(struct drm_connector *connector) { - drm_connector_update_edid_property(connector, &gm12u320_edid); - return drm_add_edid_modes(connector, &gm12u320_edid); + const struct drm_edid *drm_edid; + int count; + + drm_edid = drm_edid_alloc(&gm12u320_edid, sizeof(gm12u320_edid)); + drm_edid_connector_update(connector, drm_edid); + count = drm_edid_connector_add_modes(connector); + drm_edid_free(drm_edid); + + return count; } static const struct drm_connector_helper_funcs gm12u320_conn_helper_funcs = {
Prefer the struct drm_edid based functions for allocating the EDID and updating the connector. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- Cc: Hans de Goede <hdegoede@redhat.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <mripard@kernel.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/tiny/gm12u320.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)