[PATCH 7/8] drm/display: Move HDMI helpers into display-helper module

[Thomas Zimmermann]

Hi

Am 07.04.22 um 09:34 schrieb Jani Nikula:
> On Thu, 07 Apr 2022, Jani Nikula <jani.nikula at linux.intel.com> wrote:
>> On Wed, 06 Apr 2022, Thomas Zimmermann <tzimmermann at suse.de> wrote:
>>> Hi
>>>
>>> Am 30.03.22 um 12:35 schrieb Jani Nikula:
>>>> On Tue, 22 Mar 2022, Thomas Zimmermann <tzimmermann at 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.

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
>>> Thomas
>>>>
>>>>
>>>> BR,
>>>> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220407/d695b357/attachment-0001.sig>