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) >
