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 <mrip...@kernel.org>
> ---
>  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)
>

Reply via email to