On 09/05/2025, Dmitry Baryshkov wrote: > On Thu, Sep 04, 2025 at 05:10:02PM +0800, Liu Ying wrote: >> IT6263 supports HDMI vendor specific infoframe. The infoframe header >> and payload are configurable via NULL packet registers. The infoframe >> is enabled and disabled via PKT_NULL_CTRL register. Add the HDMI vendor >> specific infoframe support. >> >> Signed-off-by: Liu Ying <[email protected]> >> --- >> drivers/gpu/drm/bridge/ite-it6263.c | 72 >> ++++++++++++++++++++++++++----------- >> 1 file changed, 52 insertions(+), 20 deletions(-) >> >> + case HDMI_INFOFRAME_TYPE_VENDOR: >> + const char zero_bulk[HDMI_PKT_HB_PB_CHUNK_SIZE] = { }; >> + >> + /* clear NULL packet registers due to undefined default value */ >> + regmap_bulk_write(regmap, HDMI_REG_PKT_HB(0), >> + zero_bulk, sizeof(zero_bulk)); > > What if you move this to the probe function? Then there will be no need > to write those registers each time the infoframe is being written.
Good idea. But looking at drm_hdmi_vendor_infoframe_from_display_mode(), hdmi_vendor_infoframe_length() and hdmi_vendor_infoframe_pack_only(), the payload length could be changed in runtime according to display mode's VIC and flags(see DRM_MODE_FLAG_3D_MASK). And, IT6263 supports HDMI1.4a 3D formats according to it's product information[1]. So, it makes sense to clear HDMI_REG_PKT_PB(5) and HDMI_REG_PKT_PB(6) here which map to ptr[8] and ptr[9] in hdmi_vendor_infoframe_pack_only(). For v2, I'd move the NULL packet registers bulk write to it6263_hdmi_config()(i.e., it6263_probe()) and write zero to HDMI_REG_PKT_PB(5) and HDMI_REG_PKT_PB(6) here. What do you think? [1] www.ite.com.tw/en/product/cate1/IT6263 > > LGTM otherwise. > >> + >> + /* write header and payload */ >> + regmap_bulk_write(regmap, HDMI_REG_PKT_HB(0), buffer, len); >> + >> + regmap_write(regmap, HDMI_REG_PKT_NULL_CTRL, >> + ENABLE_PKT | REPEAT_PKT); >> + break; > -- Regards, Liu Ying
