Hi Am 07.04.22 um 09:34 schrieb Jani Nikula:
On Thu, 07 Apr 2022, Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:On Wed, 06 Apr 2022, Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:Hi Am 30.03.22 um 12:35 schrieb Jani Nikula:On Tue, 22 Mar 2022, Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 144c495b99c4..e6e9e4557067 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -391,33 +391,6 @@ drm_load_edid_firmware(struct drm_connector *connector)bool drm_edid_are_equal(const struct edid *edid1, const struct edid *edid2); -int-drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, - const struct drm_connector *connector, - const struct drm_display_mode *mode); -int -drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame, - const struct drm_connector *connector, - const struct drm_display_mode *mode); - -void -drm_hdmi_avi_infoframe_colorimetry(struct hdmi_avi_infoframe *frame, - const struct drm_connector_state *conn_state); - -void -drm_hdmi_avi_infoframe_bars(struct hdmi_avi_infoframe *frame, - const struct drm_connector_state *conn_state); - -void -drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame, - const struct drm_connector *connector, - const struct drm_display_mode *mode, - enum hdmi_quantization_range rgb_quant_range); - -int -drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame, - const struct drm_connector_state *conn_state); - /** * drm_eld_mnl - Get ELD monitor name length in bytes. * @eld: pointer to an eld memory structure with mnl set @@ -587,6 +560,10 @@ void drm_edid_get_monitor_name(struct edid *edid, char *name, struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev, int hsize, int vsize, int fresh, bool rb); + +u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match); +enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code); +enum hdmi_picture_aspect drm_get_hdmi_aspect_ratio(const u8 video_code);I think these were fine as static, but not really great interfaces to export. There's zero input checking on the vic in the latter, because internally we could be sure they were fine.I see. If nothing else, HDMI could be removed from the patchset. OTOH having these HDMI functions as part of the edid code doesn't seem right either.To clarify, I think the HDMI functionality should probably be moved. It's just the new interfaces I'm worried about. BR, Jani.I also wish we could limit the usage to the module you're adding; this is now available to all drivers which should be discouraged.Why is that discouraged? Quite a few drivers use these interfaces.No driver needed to directly use the functions you're now additionally exporting from drm_edid.c. I'd hope no driver starts to use them either.
I see. I'll reduce the code in the display library, even if that means that some potential helper remains in drm_edid.c for now.
Best regards Thomas
BR, Jani.Best regards ThomasBR, Jani.
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature