On Thu, 07 Apr 2022, Jani Nikula <jani.nik...@linux.intel.com> wrote: > On Wed, 06 Apr 2022, Thomas Zimmermann <tzimmerm...@suse.de> wrote: >> Hi >> >> Am 30.03.22 um 12:35 schrieb Jani Nikula: >>> On Tue, 22 Mar 2022, Thomas Zimmermann <tzimmerm...@suse.de> 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. > > BR, > Jani. > > > >> >> Best regards >> Thomas >>> >>> >>> BR, >>> Jani. >>> >>> -- Jani Nikula, Intel Open Source Graphics Center
