Re: [PATCH] drm/bridge: ite-it6263: Support HDMI vendor specific infoframe

[Liu Ying <victor.liu@nxp.com>]

On 09/05/2025, Dmitry Baryshkov wrote:
> On Fri, Sep 05, 2025 at 01:46:56PM +0800, Liu Ying wrote:
>> 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 <victor.liu@nxp.com>
>>>> ---
>>>>  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.
> 
> Then you don't even need to write zeroes in probe(). Just write
> something like:
> 
> regmap_bulk_write(regmap, HDMI_REG_PKT_HB(len), zero_bulk, FRAMESIZE-len);
> 
> But as a note: I don't think other drivers zero out packet memory. I
> think it's expected that displays ignore the frame after the 'len'
> bytes.

Then I'd choose not to zero out any packet memory since it turns out at
least for my setup that the vendor specific infoframe can be captured by
HDMI analyzer without doing that(HDMI analyzer does show leftover random
packet data).

> 
>>
>> What do you think?
>>
>> [1] http://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
> 


-- 
Regards,
Liu Ying