Re: [PATCH v4 38/45] drm/rockchip: inno_hdmi: Switch to infoframe type

From: Johan Jonker
Date: Tue Nov 28 2023 - 06:45:12 EST

Hi,

Maximum for inno_hdmi is: 1920x1080.

Do we still need INFOFRAME_VSI?

>From Rockchip RK3036 TRM V1.0 20150907-Part2 Peripheral and Interface.pdf:

- HDMI 1.4a/b/1.3/1.2/1.1, HDCP 1.2 and DVI 1.0 standard compliant transmitter
- Supports all DTV resolutions including 480p/576p/720p/1080p

https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_edid.c#L7015

* HDMI spec says if a mode is found in HDMI 1.4b 4K modes
* we should send its VIC in vendor infoframes, else send the
* VIC in AVI infoframes. Lets check if this mode is present in
* HDMI 1.4b 4K modes https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_edid.c#L7212 * Note that there's is a need to send HDMI vendor infoframes only when using a
* 4k or stereoscopic 3D mode. So when giving any other mode as input this
* function will return -EINVAL, error that can be safely ignored.


On 11/28/23 11:24, Maxime Ripard wrote:
> The inno_hdmi driver relies on its own internal infoframe type matching
> the hardware.
>
> This works fine, but in order to make further reworks easier, let's
> switch to the HDMI spec definition of those types.
>
> Signed-off-by: Maxime Ripard <mripard@xxxxxxxxxx>
> ---
> drivers/gpu/drm/rockchip/inno_hdmi.c | 71 +++++++++++++++++++++++-------------
> 1 file changed, 45 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
> index bc7fb1278cb2..ed1d10efbef4 100644
> --- a/drivers/gpu/drm/rockchip/inno_hdmi.c
> +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
> @@ -156,61 +156,80 @@ static void inno_hdmi_reset(struct inno_hdmi *hdmi)
> inno_hdmi_set_pwr_mode(hdmi, NORMAL);
> }
>
> +static u32 inno_hdmi_get_frame_index(struct inno_hdmi *hdmi,
> + enum hdmi_infoframe_type type)
> +{
> + struct drm_device *drm = hdmi->connector.dev;
> +
> + switch (type) {
> + case HDMI_INFOFRAME_TYPE_VENDOR:
> + return INFOFRAME_VSI;
> + case HDMI_INFOFRAME_TYPE_AVI:
> + return INFOFRAME_AVI;
> + default:
> + drm_err(drm, "Unknown infoframe type: %u\n", type);
> + }
> +
> + return 0;
> +}
> +
> static u32 inno_hdmi_get_frame_mask(struct inno_hdmi *hdmi,
> - u32 frame_index)
> + enum hdmi_infoframe_type type)
> {
> struct drm_device *drm = hdmi->connector.dev;
>
> - switch (frame_index) {
> - case INFOFRAME_VSI:
> + switch (type) {
> + case HDMI_INFOFRAME_TYPE_VENDOR:
> return m_PACKET_VSI_EN;
> - case INFOFRAME_AVI:
> + case HDMI_INFOFRAME_TYPE_AVI:
> return 0;
> default:
> - drm_err(drm, "Unknown infoframe type: %u\n", frame_index);
> + drm_err(drm, "Unknown infoframe type: %u\n", type);
> }
>
> return 0;
> }
>
> static u32 inno_hdmi_get_frame_disable(struct inno_hdmi *hdmi,
> - u32 frame_index)
> + enum hdmi_infoframe_type type)
> {
> struct drm_device *drm = hdmi->connector.dev;
>
> - switch (frame_index) {
> - case INFOFRAME_VSI:
> + switch (type) {
> + case HDMI_INFOFRAME_TYPE_VENDOR:
> return v_PACKET_VSI_EN(0);
> - case INFOFRAME_AVI:
> + case HDMI_INFOFRAME_TYPE_AVI:
> return 0;
> default:
> - drm_err(drm, "Unknown infoframe type: %u\n", frame_index);
> + drm_err(drm, "Unknown infoframe type: %u\n", type);
> }
>
> return 0;
> }
>
> static u32 inno_hdmi_get_frame_enable(struct inno_hdmi *hdmi,
> - u32 frame_index)
> + enum hdmi_infoframe_type type)
> {
> struct drm_device *drm = hdmi->connector.dev;
>
> - switch (frame_index) {
> - case INFOFRAME_VSI:
> + switch (type) {
> + case HDMI_INFOFRAME_TYPE_VENDOR:
> return v_PACKET_VSI_EN(1);
> - case INFOFRAME_AVI:
> + case HDMI_INFOFRAME_TYPE_AVI:
> return 0;
> default:
> - drm_err(drm, "Unknown infoframe type: %u\n", frame_index);
> + drm_err(drm, "Unknown infoframe type: %u\n", type);
> }
>
> return 0;
> }
>
> -static void inno_hdmi_disable_frame(struct inno_hdmi *hdmi, u32 frame_index)
> +static void inno_hdmi_disable_frame(struct inno_hdmi *hdmi,
> + enum hdmi_infoframe_type type)
> {
> - u32 disable = inno_hdmi_get_frame_disable(hdmi, frame_index);
> - u32 mask = inno_hdmi_get_frame_mask(hdmi, frame_index);
> + u32 frame_index = inno_hdmi_get_frame_index(hdmi, type);
> + u32 disable = inno_hdmi_get_frame_disable(hdmi, type);
> + u32 mask = inno_hdmi_get_frame_mask(hdmi, type);
>
> if (mask)
> hdmi_modb(hdmi, HDMI_PACKET_SEND_AUTO, mask, disable);
> @@ -219,14 +238,14 @@ static void inno_hdmi_disable_frame(struct inno_hdmi *hdmi, u32 frame_index)
> }
>
> static int inno_hdmi_upload_frame(struct inno_hdmi *hdmi,
> - union hdmi_infoframe *frame, u32 frame_index)
> + union hdmi_infoframe *frame, enum hdmi_infoframe_type type)
> {
> - u32 enable = inno_hdmi_get_frame_enable(hdmi, frame_index);
> - u32 mask = inno_hdmi_get_frame_mask(hdmi, frame_index);
> + u32 enable = inno_hdmi_get_frame_enable(hdmi, type);
> + u32 mask = inno_hdmi_get_frame_mask(hdmi, type);
> u8 packed_frame[HDMI_MAXIMUM_INFO_FRAME_SIZE];
> ssize_t rc, i;
>
> - inno_hdmi_disable_frame(hdmi, frame_index);
> + inno_hdmi_disable_frame(hdmi, type);
>
> rc = hdmi_infoframe_pack(frame, packed_frame,
> sizeof(packed_frame));
> @@ -253,11 +272,11 @@ static int inno_hdmi_config_video_vsi(struct inno_hdmi *hdmi,
> &hdmi->connector,
> mode);
> if (rc) {
> - inno_hdmi_disable_frame(hdmi, INFOFRAME_VSI);
> + inno_hdmi_disable_frame(hdmi, HDMI_INFOFRAME_TYPE_VENDOR);
> return rc;
> }
>
> - return inno_hdmi_upload_frame(hdmi, &frame, INFOFRAME_VSI);
> + return inno_hdmi_upload_frame(hdmi, &frame, HDMI_INFOFRAME_TYPE_VENDOR);
> }
>
> static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi,
> @@ -270,13 +289,13 @@ static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi,
> &hdmi->connector,
> mode);
> if (rc) {
> - inno_hdmi_disable_frame(hdmi, INFOFRAME_AVI);
> + inno_hdmi_disable_frame(hdmi, HDMI_INFOFRAME_TYPE_AVI);
> return rc;
> }
>
> frame.avi.colorspace = HDMI_COLORSPACE_RGB;
>
> - return inno_hdmi_upload_frame(hdmi, &frame, INFOFRAME_AVI);
> + return inno_hdmi_upload_frame(hdmi, &frame, HDMI_INFOFRAME_TYPE_AVI);
> }
>
> static int inno_hdmi_config_video_csc(struct inno_hdmi *hdmi)
>