| Message ID | 9fa366147b06a28304527be48f1b363c3484c8a3.1713259151.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/edid: cleanups, rebase | expand |
Hi Am 16.04.24 um 11:19 schrieb Jani Nikula: > Prefer the EDID iterators over drm_edid_find_extension() in > drm_edid_has_cta_extension(), even if this leads to more code. The key > is to use the same patterns as much as possible. Should this patch go before patch 4? That would limit the impact of the latter. Why is this instance different than the one in drm_find_displayid_extension()? Best regards Thomas > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/drm_edid.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index c29f31dcc818..4b3ad42a8f95 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -4230,11 +4230,21 @@ static bool drm_edid_has_cta_extension(const struct drm_edid *drm_edid) > { > const struct displayid_block *block; > struct displayid_iter iter; > - int ext_index = 0; > + struct drm_edid_iter edid_iter; > + const u8 *ext; > bool found = false; > > /* Look for a top level CEA extension block */ > - if (drm_edid_find_extension(drm_edid, CEA_EXT, &ext_index)) > + drm_edid_iter_begin(drm_edid, &edid_iter); > + drm_edid_iter_for_each(ext, &edid_iter) { > + if (ext[0] == CEA_EXT) { > + found = true; > + break; > + } > + } > + drm_edid_iter_end(&edid_iter); > + > + if (found) > return true; > > /* CEA blocks can also be found embedded in a DisplayID block */
On Tue, 16 Apr 2024, Thomas Zimmermann <tzimmermann@suse.de> wrote: > Hi > > Am 16.04.24 um 11:19 schrieb Jani Nikula: >> Prefer the EDID iterators over drm_edid_find_extension() in >> drm_edid_has_cta_extension(), even if this leads to more code. The key >> is to use the same patterns as much as possible. > > Should this patch go before patch 4? That would limit the impact of the > latter. I can if you want, IMO not a big deal. > Why is this instance different than the one in > drm_find_displayid_extension()? Best regards Thomas Overall I'd like to get rid of the function altogether, but I'm undecided what the replacement interface towards drm_displayid.c should be. Maybe expose the drm_edid_iter_* stuff? But I really don't want anyone to export and start using them in drivers. BR, Jani. >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> drivers/gpu/drm/drm_edid.c | 14 ++++++++++++-- >> 1 file changed, 12 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >> index c29f31dcc818..4b3ad42a8f95 100644 >> --- a/drivers/gpu/drm/drm_edid.c >> +++ b/drivers/gpu/drm/drm_edid.c >> @@ -4230,11 +4230,21 @@ static bool drm_edid_has_cta_extension(const struct drm_edid *drm_edid) >> { >> const struct displayid_block *block; >> struct displayid_iter iter; >> - int ext_index = 0; >> + struct drm_edid_iter edid_iter; >> + const u8 *ext; >> bool found = false; >> >> /* Look for a top level CEA extension block */ >> - if (drm_edid_find_extension(drm_edid, CEA_EXT, &ext_index)) >> + drm_edid_iter_begin(drm_edid, &edid_iter); >> + drm_edid_iter_for_each(ext, &edid_iter) { >> + if (ext[0] == CEA_EXT) { >> + found = true; >> + break; >> + } >> + } >> + drm_edid_iter_end(&edid_iter); >> + >> + if (found) >> return true; >> >> /* CEA blocks can also be found embedded in a DisplayID block */
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c29f31dcc818..4b3ad42a8f95 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4230,11 +4230,21 @@ static bool drm_edid_has_cta_extension(const struct drm_edid *drm_edid) { const struct displayid_block *block; struct displayid_iter iter; - int ext_index = 0; + struct drm_edid_iter edid_iter; + const u8 *ext; bool found = false; /* Look for a top level CEA extension block */ - if (drm_edid_find_extension(drm_edid, CEA_EXT, &ext_index)) + drm_edid_iter_begin(drm_edid, &edid_iter); + drm_edid_iter_for_each(ext, &edid_iter) { + if (ext[0] == CEA_EXT) { + found = true; + break; + } + } + drm_edid_iter_end(&edid_iter); + + if (found) return true; /* CEA blocks can also be found embedded in a DisplayID block */
Prefer the EDID iterators over drm_edid_find_extension() in drm_edid_has_cta_extension(), even if this leads to more code. The key is to use the same patterns as much as possible. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/drm_edid.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)