| Message ID | 386e3a64efbdd61c3eaed3f49ea9c3ebd4fcd41d.1715691257.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: conversions to struct drm_edid | expand |
Hi Am 14.05.24 um 14:55 schrieb Jani Nikula: > Prefer the struct drm_edid based functions for reading the EDID and > updating the connector. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > --- > > Cc: Xinliang Liu <xinliang.liu@linaro.org> > Cc: Tian Tao <tiantao6@hisilicon.com> > Cc: Xinwei Kong <kong.kongxinwei@hisilicon.com> > Cc: Sumit Semwal <sumit.semwal@linaro.org> > Cc: Yongqin Liu <yongqin.liu@linaro.org> > Cc: John Stultz <jstultz@google.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > --- > .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 17 ++++++++++------- > 1 file changed, 10 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c > index 94e2c573a7af..409c551c92af 100644 > --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c > +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c > @@ -24,14 +24,16 @@ > > static int hibmc_connector_get_modes(struct drm_connector *connector) Could this function be replaced with a call to drm_connector_helper_get_modes()? With the patch, the only difference is the call to the _noedid() function, which the DRM core does automatically. There will still be a difference in the maximum resolution, but standardizing on 1024x768 seems preferable to me. Best regards Thomas > { > - int count; > - void *edid; > struct hibmc_connector *hibmc_connector = to_hibmc_connector(connector); > + const struct drm_edid *drm_edid; > + int count; > + > + drm_edid = drm_edid_read_ddc(connector, &hibmc_connector->adapter); > > - edid = drm_get_edid(connector, &hibmc_connector->adapter); > - if (edid) { > - drm_connector_update_edid_property(connector, edid); > - count = drm_add_edid_modes(connector, edid); > + drm_edid_connector_update(connector, drm_edid); > + > + if (drm_edid) { > + count = drm_edid_connector_add_modes(connector); > if (count) > goto out; > } > @@ -42,7 +44,8 @@ static int hibmc_connector_get_modes(struct drm_connector *connector) > drm_set_preferred_mode(connector, 1024, 768); > > out: > - kfree(edid); > + drm_edid_free(drm_edid); > + > return count; > } >
On Tue, 14 May 2024, Thomas Zimmermann <tzimmermann@suse.de> wrote: > Hi > > Am 14.05.24 um 14:55 schrieb Jani Nikula: >> Prefer the struct drm_edid based functions for reading the EDID and >> updating the connector. >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> >> --- >> >> Cc: Xinliang Liu <xinliang.liu@linaro.org> >> Cc: Tian Tao <tiantao6@hisilicon.com> >> Cc: Xinwei Kong <kong.kongxinwei@hisilicon.com> >> Cc: Sumit Semwal <sumit.semwal@linaro.org> >> Cc: Yongqin Liu <yongqin.liu@linaro.org> >> Cc: John Stultz <jstultz@google.com> >> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> Cc: Maxime Ripard <mripard@kernel.org> >> Cc: Thomas Zimmermann <tzimmermann@suse.de> >> --- >> .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 17 ++++++++++------- >> 1 file changed, 10 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c >> index 94e2c573a7af..409c551c92af 100644 >> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c >> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c >> @@ -24,14 +24,16 @@ >> >> static int hibmc_connector_get_modes(struct drm_connector *connector) > > Could this function be replaced with a call to > drm_connector_helper_get_modes()? With the patch, the only difference is > the call to the _noedid() function, which the DRM core does > automatically. There will still be a difference in the maximum > resolution, but standardizing on 1024x768 seems preferable to me. Looks like it might work, but personally, I'm trying to shy away from any further cleanups than just switching to struct drm_edid. I've got my plate pretty full already. BR, Jani. > > Best regards > Thomas > >> { >> - int count; >> - void *edid; >> struct hibmc_connector *hibmc_connector = to_hibmc_connector(connector); >> + const struct drm_edid *drm_edid; >> + int count; >> + >> + drm_edid = drm_edid_read_ddc(connector, &hibmc_connector->adapter); >> >> - edid = drm_get_edid(connector, &hibmc_connector->adapter); >> - if (edid) { >> - drm_connector_update_edid_property(connector, edid); >> - count = drm_add_edid_modes(connector, edid); >> + drm_edid_connector_update(connector, drm_edid); >> + >> + if (drm_edid) { >> + count = drm_edid_connector_add_modes(connector); >> if (count) >> goto out; >> } >> @@ -42,7 +44,8 @@ static int hibmc_connector_get_modes(struct drm_connector *connector) >> drm_set_preferred_mode(connector, 1024, 768); >> >> out: >> - kfree(edid); >> + drm_edid_free(drm_edid); >> + >> return count; >> } >>
Hi Am 15.05.24 um 14:34 schrieb Jani Nikula: > On Tue, 14 May 2024, Thomas Zimmermann <tzimmermann@suse.de> wrote: >> Hi >> >> Am 14.05.24 um 14:55 schrieb Jani Nikula: >>> Prefer the struct drm_edid based functions for reading the EDID and >>> updating the connector. >>> >>> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >>> >>> --- >>> >>> Cc: Xinliang Liu <xinliang.liu@linaro.org> >>> Cc: Tian Tao <tiantao6@hisilicon.com> >>> Cc: Xinwei Kong <kong.kongxinwei@hisilicon.com> >>> Cc: Sumit Semwal <sumit.semwal@linaro.org> >>> Cc: Yongqin Liu <yongqin.liu@linaro.org> >>> Cc: John Stultz <jstultz@google.com> >>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >>> Cc: Maxime Ripard <mripard@kernel.org> >>> Cc: Thomas Zimmermann <tzimmermann@suse.de> >>> --- >>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 17 ++++++++++------- >>> 1 file changed, 10 insertions(+), 7 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c >>> index 94e2c573a7af..409c551c92af 100644 >>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c >>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c >>> @@ -24,14 +24,16 @@ >>> >>> static int hibmc_connector_get_modes(struct drm_connector *connector) >> Could this function be replaced with a call to >> drm_connector_helper_get_modes()? With the patch, the only difference is >> the call to the _noedid() function, which the DRM core does >> automatically. There will still be a difference in the maximum >> resolution, but standardizing on 1024x768 seems preferable to me. > Looks like it might work, but personally, I'm trying to shy away from > any further cleanups than just switching to struct drm_edid. I've got my > plate pretty full already. In that case Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> for the current patch. Best regards Thomas > > BR, > Jani. > > >> Best regards >> Thomas >> >>> { >>> - int count; >>> - void *edid; >>> struct hibmc_connector *hibmc_connector = to_hibmc_connector(connector); >>> + const struct drm_edid *drm_edid; >>> + int count; >>> + >>> + drm_edid = drm_edid_read_ddc(connector, &hibmc_connector->adapter); >>> >>> - edid = drm_get_edid(connector, &hibmc_connector->adapter); >>> - if (edid) { >>> - drm_connector_update_edid_property(connector, edid); >>> - count = drm_add_edid_modes(connector, edid); >>> + drm_edid_connector_update(connector, drm_edid); >>> + >>> + if (drm_edid) { >>> + count = drm_edid_connector_add_modes(connector); >>> if (count) >>> goto out; >>> } >>> @@ -42,7 +44,8 @@ static int hibmc_connector_get_modes(struct drm_connector *connector) >>> drm_set_preferred_mode(connector, 1024, 768); >>> >>> out: >>> - kfree(edid); >>> + drm_edid_free(drm_edid); >>> + >>> return count; >>> } >>>
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c index 94e2c573a7af..409c551c92af 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c @@ -24,14 +24,16 @@ static int hibmc_connector_get_modes(struct drm_connector *connector) { - int count; - void *edid; struct hibmc_connector *hibmc_connector = to_hibmc_connector(connector); + const struct drm_edid *drm_edid; + int count; + + drm_edid = drm_edid_read_ddc(connector, &hibmc_connector->adapter); - edid = drm_get_edid(connector, &hibmc_connector->adapter); - if (edid) { - drm_connector_update_edid_property(connector, edid); - count = drm_add_edid_modes(connector, edid); + drm_edid_connector_update(connector, drm_edid); + + if (drm_edid) { + count = drm_edid_connector_add_modes(connector); if (count) goto out; } @@ -42,7 +44,8 @@ static int hibmc_connector_get_modes(struct drm_connector *connector) drm_set_preferred_mode(connector, 1024, 768); out: - kfree(edid); + drm_edid_free(drm_edid); + return count; }
Prefer the struct drm_edid based functions for reading the EDID and updating the connector. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- Cc: Xinliang Liu <xinliang.liu@linaro.org> Cc: Tian Tao <tiantao6@hisilicon.com> Cc: Xinwei Kong <kong.kongxinwei@hisilicon.com> Cc: Sumit Semwal <sumit.semwal@linaro.org> Cc: Yongqin Liu <yongqin.liu@linaro.org> Cc: John Stultz <jstultz@google.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <mripard@kernel.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> --- .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)