| Message ID | 20211016184226.3862-2-cssk@net-c.es (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi | expand |
On Sat, Oct 16, 2021 at 08:42:14PM +0200, Claudio Suarez wrote: > According to the documentation, drm_add_edid_modes > "... Also fills out the &drm_display_info structure and ELD in @connector > with any information which can be derived from the edid." > > drm_add_edid_modes accepts a struct edid *edid parameter which may have a > value or may be null. When it is not null, connector->display_info and > connector->eld are updated according to the edid. When edid=NULL, only > connector->eld is reset. Reset connector->display_info to be consistent > and accurate. > > Signed-off-by: Claudio Suarez <cssk@net-c.es> > --- > drivers/gpu/drm/drm_edid.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 6325877c5fd6..c643db17782c 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -5356,14 +5356,13 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) > int num_modes = 0; > u32 quirks; > > - if (edid == NULL) { > - clear_eld(connector); > - return 0; > - } > if (!drm_edid_is_valid(edid)) { OK, so drm_edid_is_valid() will happily accept NULL and considers it invalid. You may want to mention that explicitly in the commit message. > + /* edid == NULL or invalid here */ > clear_eld(connector); > - drm_warn(connector->dev, "%s: EDID invalid.\n", > - connector->name); > + drm_reset_display_info(connector); > + if (edid) > + drm_warn(connector->dev, "%s: EDID invalid.\n", > + connector->name); Could you respin this to use the standard [CONNECTOR:%d:%s] form while at it? Or I guess a patch to mass convert the whole drm_edid.c might be another option. Patch looks good. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > return 0; > } > > -- > 2.33.0 > >
On Tue, Oct 19, 2021 at 09:35:08PM +0300, Ville Syrjälä wrote: > On Sat, Oct 16, 2021 at 08:42:14PM +0200, Claudio Suarez wrote: > > According to the documentation, drm_add_edid_modes > > "... Also fills out the &drm_display_info structure and ELD in @connector > > with any information which can be derived from the edid." > > > > drm_add_edid_modes accepts a struct edid *edid parameter which may have a > > value or may be null. When it is not null, connector->display_info and > > connector->eld are updated according to the edid. When edid=NULL, only > > connector->eld is reset. Reset connector->display_info to be consistent > > and accurate. > > > > Signed-off-by: Claudio Suarez <cssk@net-c.es> > > --- > > drivers/gpu/drm/drm_edid.c | 11 +++++------ > > 1 file changed, 5 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > > index 6325877c5fd6..c643db17782c 100644 > > --- a/drivers/gpu/drm/drm_edid.c > > +++ b/drivers/gpu/drm/drm_edid.c > > @@ -5356,14 +5356,13 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) > > int num_modes = 0; > > u32 quirks; > > > > - if (edid == NULL) { > > - clear_eld(connector); > > - return 0; > > - } > > if (!drm_edid_is_valid(edid)) { > > OK, so drm_edid_is_valid() will happily accept NULL and considers > it invalid. You may want to mention that explicitly in the commit > message. Thank you for your comments, I appreciate :) I'm sending new mails with the new commit messages. > > + /* edid == NULL or invalid here */ > > clear_eld(connector); > > - drm_warn(connector->dev, "%s: EDID invalid.\n", > > - connector->name); > > + drm_reset_display_info(connector); > > + if (edid) > > + drm_warn(connector->dev, "%s: EDID invalid.\n", > > + connector->name); > > Could you respin this to use the standard [CONNECTOR:%d:%s] form > while at it? Or I guess a patch to mass convert the whole drm_edid.c > might be another option. Good point. I like the idea of a new patch. I'll start working on it. I can change this drm_warn here to avoid merge conflicts. > Patch looks good. > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Thanks! BR Claudio Suarez.
According to the documentation, drm_add_edid_modes
"... Also fills out the &drm_display_info structure and ELD in @connector
with any information which can be derived from the edid."
drm_add_edid_modes accepts a struct edid *edid parameter which may have a
value or may be null. When it is not null, connector->display_info and
connector->eld are updated according to the edid. When edid=NULL, only
connector->eld is reset. Reset connector->display_info to be consistent
and accurate.
Since drm_edid_is_valid() considers NULL as an invalid EDID, simplify the
code to avoid duplicating code in the case of NULL/error.
Signed-off-by: Claudio Suarez <cssk@net-c.es>
---
drivers/gpu/drm/drm_edid.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 6325877c5fd6..a019a26ede7a 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5356,14 +5356,14 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
int num_modes = 0;
u32 quirks;
- if (edid == NULL) {
- clear_eld(connector);
- return 0;
- }
if (!drm_edid_is_valid(edid)) {
+ /* edid == NULL or invalid here */
clear_eld(connector);
- drm_warn(connector->dev, "%s: EDID invalid.\n",
- connector->name);
+ drm_reset_display_info(connector);
+ if (edid)
+ drm_warn(connector->dev,
+ "[CONNECTOR:%d:%s] EDID invalid.\n",
+ connector->base.id, connector->name);
return 0;
}
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 6325877c5fd6..c643db17782c 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5356,14 +5356,13 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) int num_modes = 0; u32 quirks; - if (edid == NULL) { - clear_eld(connector); - return 0; - } if (!drm_edid_is_valid(edid)) { + /* edid == NULL or invalid here */ clear_eld(connector); - drm_warn(connector->dev, "%s: EDID invalid.\n", - connector->name); + drm_reset_display_info(connector); + if (edid) + drm_warn(connector->dev, "%s: EDID invalid.\n", + connector->name); return 0; }
According to the documentation, drm_add_edid_modes "... Also fills out the &drm_display_info structure and ELD in @connector with any information which can be derived from the edid." drm_add_edid_modes accepts a struct edid *edid parameter which may have a value or may be null. When it is not null, connector->display_info and connector->eld are updated according to the edid. When edid=NULL, only connector->eld is reset. Reset connector->display_info to be consistent and accurate. Signed-off-by: Claudio Suarez <cssk@net-c.es> --- drivers/gpu/drm/drm_edid.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)