| Message ID | 20240102111921.3057255-1-javierm@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm: Move drm_set_preferred_mode() helper from drm_edid to drm_modes | expand |
On Tue, 02 Jan 2024, Javier Martinez Canillas <javierm@redhat.com> 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@redhat.com> > --- > > 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. Other than that, Reviewed-by: Jani Nikula <jani.nikula@intel.com> > #include <drm/drm_print.h> > > #include "drm_crtc_internal.h" > @@ -6989,28 +6990,6 @@ int drm_add_modes_noedid(struct drm_connector *connector, > } > EXPORT_SYMBOL(drm_add_modes_noedid); > > -/** > - * drm_set_preferred_mode - Sets the preferred mode of a connector > - * @connector: connector whose mode list should be processed > - * @hpref: horizontal resolution of preferred mode > - * @vpref: vertical resolution of preferred mode > - * > - * Marks a mode as preferred if it matches the resolution specified by @hpref > - * and @vpref. > - */ > -void drm_set_preferred_mode(struct drm_connector *connector, > - int hpref, int vpref) > -{ > - struct drm_display_mode *mode; > - > - list_for_each_entry(mode, &connector->probed_modes, head) { > - if (mode->hdisplay == hpref && > - mode->vdisplay == vpref) > - mode->type |= DRM_MODE_TYPE_PREFERRED; > - } > -} > -EXPORT_SYMBOL(drm_set_preferred_mode); > - > static bool is_hdmi2_sink(const struct drm_connector *connector) > { > /* > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index ac9a406250c5..01aa44e87534 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -2754,3 +2754,25 @@ bool drm_mode_is_420(const struct drm_display_info *display, > drm_mode_is_420_also(display, mode); > } > EXPORT_SYMBOL(drm_mode_is_420); > + > +/** > + * drm_set_preferred_mode - Sets the preferred mode of a connector > + * @connector: connector whose mode list should be processed > + * @hpref: horizontal resolution of preferred mode > + * @vpref: vertical resolution of preferred mode > + * > + * Marks a mode as preferred if it matches the resolution specified by @hpref > + * and @vpref. > + */ > +void drm_set_preferred_mode(struct drm_connector *connector, > + int hpref, int vpref) > +{ > + struct drm_display_mode *mode; > + > + list_for_each_entry(mode, &connector->probed_modes, head) { > + if (mode->hdisplay == hpref && > + mode->vdisplay == vpref) > + mode->type |= DRM_MODE_TYPE_PREFERRED; > + } > +} > +EXPORT_SYMBOL(drm_set_preferred_mode); > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h > index 54cc6f04a708..5bd6b6eb6c57 100644 > --- a/include/drm/drm_edid.h > +++ b/include/drm/drm_edid.h > @@ -426,8 +426,6 @@ enum hdmi_quantization_range > drm_default_rgb_quant_range(const struct drm_display_mode *mode); > int drm_add_modes_noedid(struct drm_connector *connector, > int hdisplay, int vdisplay); > -void drm_set_preferred_mode(struct drm_connector *connector, > - int hpref, int vpref); > > int drm_edid_header_is_valid(const void *edid); > bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid, > diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h > index c613f0abe9dc..b9bb92e4b029 100644 > --- a/include/drm/drm_modes.h > +++ b/include/drm/drm_modes.h > @@ -467,6 +467,8 @@ bool drm_mode_is_420_also(const struct drm_display_info *display, > const struct drm_display_mode *mode); > bool drm_mode_is_420(const struct drm_display_info *display, > const struct drm_display_mode *mode); > +void drm_set_preferred_mode(struct drm_connector *connector, > + int hpref, int vpref); > > struct drm_display_mode *drm_analog_tv_mode(struct drm_device *dev, > enum drm_connector_tv_mode mode,
Jani Nikula <jani.nikula@linux.intel.com> writes: Hello Jani, > On Tue, 02 Jan 2024, Javier Martinez Canillas <javierm@redhat.com> 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@redhat.com> >> --- >> >> 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@intel.com> > Thanks. I'll post a v2 that drops the unnecessary header include.
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> #include <drm/drm_print.h> #include "drm_crtc_internal.h" @@ -6989,28 +6990,6 @@ int drm_add_modes_noedid(struct drm_connector *connector, } EXPORT_SYMBOL(drm_add_modes_noedid); -/** - * drm_set_preferred_mode - Sets the preferred mode of a connector - * @connector: connector whose mode list should be processed - * @hpref: horizontal resolution of preferred mode - * @vpref: vertical resolution of preferred mode - * - * Marks a mode as preferred if it matches the resolution specified by @hpref - * and @vpref. - */ -void drm_set_preferred_mode(struct drm_connector *connector, - int hpref, int vpref) -{ - struct drm_display_mode *mode; - - list_for_each_entry(mode, &connector->probed_modes, head) { - if (mode->hdisplay == hpref && - mode->vdisplay == vpref) - mode->type |= DRM_MODE_TYPE_PREFERRED; - } -} -EXPORT_SYMBOL(drm_set_preferred_mode); - static bool is_hdmi2_sink(const struct drm_connector *connector) { /* diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index ac9a406250c5..01aa44e87534 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -2754,3 +2754,25 @@ bool drm_mode_is_420(const struct drm_display_info *display, drm_mode_is_420_also(display, mode); } EXPORT_SYMBOL(drm_mode_is_420); + +/** + * drm_set_preferred_mode - Sets the preferred mode of a connector + * @connector: connector whose mode list should be processed + * @hpref: horizontal resolution of preferred mode + * @vpref: vertical resolution of preferred mode + * + * Marks a mode as preferred if it matches the resolution specified by @hpref + * and @vpref. + */ +void drm_set_preferred_mode(struct drm_connector *connector, + int hpref, int vpref) +{ + struct drm_display_mode *mode; + + list_for_each_entry(mode, &connector->probed_modes, head) { + if (mode->hdisplay == hpref && + mode->vdisplay == vpref) + mode->type |= DRM_MODE_TYPE_PREFERRED; + } +} +EXPORT_SYMBOL(drm_set_preferred_mode); diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 54cc6f04a708..5bd6b6eb6c57 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -426,8 +426,6 @@ enum hdmi_quantization_range drm_default_rgb_quant_range(const struct drm_display_mode *mode); int drm_add_modes_noedid(struct drm_connector *connector, int hdisplay, int vdisplay); -void drm_set_preferred_mode(struct drm_connector *connector, - int hpref, int vpref); int drm_edid_header_is_valid(const void *edid); bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid, diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index c613f0abe9dc..b9bb92e4b029 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -467,6 +467,8 @@ bool drm_mode_is_420_also(const struct drm_display_info *display, const struct drm_display_mode *mode); bool drm_mode_is_420(const struct drm_display_info *display, const struct drm_display_mode *mode); +void drm_set_preferred_mode(struct drm_connector *connector, + int hpref, int vpref); struct drm_display_mode *drm_analog_tv_mode(struct drm_device *dev, enum drm_connector_tv_mode mode,
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@redhat.com> --- 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(-)