Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> writes: Hello Jani, > On Tue, 02 Jan 2024, Javier Martinez Canillas <javierm@xxxxxxxxxx> wrote: >> The helper is generic and doesn't use the opaque EDID type struct drm_edid >> and is also used by drivers that only support non-probeable displays, such >> as fixed panels. >> >> These drivers add a list of modes using drm_mode_probed_add() and then set >> a preferred mode using the drm_set_preferred_mode() helper. >> >> It seems more logical to have the helper definition in drm_modes.o instead >> of drm_edid.o, since the former contains modes helper while the latter has >> helpers to manage the EDID information. >> >> Since both drm_edid.o and drm_modes.o object files are built-in the drm.o >> object, there are no functional changes. But besides being a more logical >> place for this helper, it could also allow to eventually make drm_edid.o >> optional and not included in drm.o if only fixed panels must be supported >> in a given system. >> >> Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx> >> --- >> >> drivers/gpu/drm/drm_edid.c | 23 +---------------------- >> drivers/gpu/drm/drm_modes.c | 22 ++++++++++++++++++++++ >> include/drm/drm_edid.h | 2 -- >> include/drm/drm_modes.h | 2 ++ >> 4 files changed, 25 insertions(+), 24 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >> index cb4031d5dcbb..48dd2a0a0395 100644 >> --- a/drivers/gpu/drm/drm_edid.c >> +++ b/drivers/gpu/drm/drm_edid.c >> @@ -43,6 +43,7 @@ >> #include <drm/drm_edid.h> >> #include <drm/drm_eld.h> >> #include <drm/drm_encoder.h> >> +#include <drm/drm_modes.h> > > Unnecessary. > Indeed. I could swear that saw drm_set_preferred_mode() being called somewhere in drm_edid.c but looking again I see that's not the case. > Other than that, > > Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx> > Thanks. I'll post a v2 that drops the unnecessary header include. -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
