[PATCH 3/7] drm/bridge: ite-it6232: declare supported infoframes

Dmitry Baryshkov posted 7 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH 3/7] drm/bridge: ite-it6232: declare supported infoframes
Posted by Dmitry Baryshkov 1 month, 2 weeks ago
Declare which infoframes are supported via the .hdmi_write_infoframe()
interface.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/bridge/ite-it6263.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/bridge/ite-it6263.c b/drivers/gpu/drm/bridge/ite-it6263.c
index cf813672b4ffb8ab5c524c6414ee7b414cebc018..b1956891a8388401c13cd2fc5c78f0779063adf4 100644
--- a/drivers/gpu/drm/bridge/ite-it6263.c
+++ b/drivers/gpu/drm/bridge/ite-it6263.c
@@ -875,6 +875,7 @@ static int it6263_probe(struct i2c_client *client)
 	it->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
 	it->bridge.vendor = "ITE";
 	it->bridge.product = "IT6263";
+	it->bridge.supported_infoframes = DRM_CONNECTOR_INFOFRAME_AVI;
 
 	return devm_drm_bridge_add(dev, &it->bridge);
 }

-- 
2.47.2
Re: [PATCH 3/7] drm/bridge: ite-it6232: declare supported infoframes
Posted by Liu Ying 1 month, 2 weeks ago
Hi Dmitry,

On 08/16/2025, Dmitry Baryshkov wrote:
> Declare which infoframes are supported via the .hdmi_write_infoframe()
> interface.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/bridge/ite-it6263.c | 1 +
>  1 file changed, 1 insertion(+)

For subject, s/it6232/it6263.

> 
> diff --git a/drivers/gpu/drm/bridge/ite-it6263.c b/drivers/gpu/drm/bridge/ite-it6263.c
> index cf813672b4ffb8ab5c524c6414ee7b414cebc018..b1956891a8388401c13cd2fc5c78f0779063adf4 100644
> --- a/drivers/gpu/drm/bridge/ite-it6263.c
> +++ b/drivers/gpu/drm/bridge/ite-it6263.c
> @@ -875,6 +875,7 @@ static int it6263_probe(struct i2c_client *client)
>  	it->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
>  	it->bridge.vendor = "ITE";
>  	it->bridge.product = "IT6263";
> +	it->bridge.supported_infoframes = DRM_CONNECTOR_INFOFRAME_AVI;

With supported_infoframes set, the two lines setting vendor and product
are dead code.  I think it's worth dropping them and updating kerneldoc
for vendor and product members because they don't have to be required if
DRM_BRIDGE_OP_HDMI is set.  But, this could be done with future patch(not
in this patch series).

Reviewed-by: Liu Ying <victor.liu@nxp.com>

>  
>  	return devm_drm_bridge_add(dev, &it->bridge);
>  }
> 


-- 
Regards,
Liu Ying
Re: [PATCH 3/7] drm/bridge: ite-it6232: declare supported infoframes
Posted by Dmitry Baryshkov 1 month, 2 weeks ago
On 19/08/2025 12:49, Liu Ying wrote:
> Hi Dmitry,
> 
> On 08/16/2025, Dmitry Baryshkov wrote:
>> Declare which infoframes are supported via the .hdmi_write_infoframe()
>> interface.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>> ---
>>   drivers/gpu/drm/bridge/ite-it6263.c | 1 +
>>   1 file changed, 1 insertion(+)
> 
> For subject, s/it6232/it6263.

Ack

> 
>>
>> diff --git a/drivers/gpu/drm/bridge/ite-it6263.c b/drivers/gpu/drm/bridge/ite-it6263.c
>> index cf813672b4ffb8ab5c524c6414ee7b414cebc018..b1956891a8388401c13cd2fc5c78f0779063adf4 100644
>> --- a/drivers/gpu/drm/bridge/ite-it6263.c
>> +++ b/drivers/gpu/drm/bridge/ite-it6263.c
>> @@ -875,6 +875,7 @@ static int it6263_probe(struct i2c_client *client)
>>   	it->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
>>   	it->bridge.vendor = "ITE";
>>   	it->bridge.product = "IT6263";
>> +	it->bridge.supported_infoframes = DRM_CONNECTOR_INFOFRAME_AVI;
> 
> With supported_infoframes set, the two lines setting vendor and product
> are dead code.  I think it's worth dropping them and updating kerneldoc
> for vendor and product members because they don't have to be required if
> DRM_BRIDGE_OP_HDMI is set.  But, this could be done with future patch(not
> in this patch series).

They are still required by the framework itself, see 
drmm_connector_hdmi_init().

BTW: I don't have ITE datasheet. Do you know if it really supports only 
the AVI frame?


> 
> Reviewed-by: Liu Ying <victor.liu@nxp.com>
> 
>>   
>>   	return devm_drm_bridge_add(dev, &it->bridge);
>>   }
>>
> 
> 


-- 
With best wishes
Dmitry
Re: [PATCH 3/7] drm/bridge: ite-it6232: declare supported infoframes
Posted by Liu Ying 1 month, 2 weeks ago
On 08/19/2025, Dmitry Baryshkov wrote:
> On 19/08/2025 12:49, Liu Ying wrote:
>> Hi Dmitry,
>>
>> On 08/16/2025, Dmitry Baryshkov wrote:
>>> Declare which infoframes are supported via the .hdmi_write_infoframe()
>>> interface.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>>> ---
>>>   drivers/gpu/drm/bridge/ite-it6263.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>
>> For subject, s/it6232/it6263.
> 
> Ack
> 
>>
>>>
>>> diff --git a/drivers/gpu/drm/bridge/ite-it6263.c b/drivers/gpu/drm/bridge/ite-it6263.c
>>> index cf813672b4ffb8ab5c524c6414ee7b414cebc018..b1956891a8388401c13cd2fc5c78f0779063adf4 100644
>>> --- a/drivers/gpu/drm/bridge/ite-it6263.c
>>> +++ b/drivers/gpu/drm/bridge/ite-it6263.c
>>> @@ -875,6 +875,7 @@ static int it6263_probe(struct i2c_client *client)
>>>       it->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
>>>       it->bridge.vendor = "ITE";
>>>       it->bridge.product = "IT6263";
>>> +    it->bridge.supported_infoframes = DRM_CONNECTOR_INFOFRAME_AVI;
>>
>> With supported_infoframes set, the two lines setting vendor and product
>> are dead code.  I think it's worth dropping them and updating kerneldoc
>> for vendor and product members because they don't have to be required if
>> DRM_BRIDGE_OP_HDMI is set.  But, this could be done with future patch(not
>> in this patch series).
> 
> They are still required by the framework itself, see drmm_connector_hdmi_init().

Yes.  But it's a bit too strict since SPD infoframe is optional according
to CTA standard documentation.

> 
> BTW: I don't have ITE datasheet. Do you know if it really supports only the AVI frame?

AFAICS, it seems that ITE6263 supports inforframes from 0x81 to 0x85, so
including SPD inforframe.  Maybe, just keep those dead vendor&product
settings for now and add SPD inforframe in future.

> 
> 
>>
>> Reviewed-by: Liu Ying <victor.liu@nxp.com>
>>
>>>         return devm_drm_bridge_add(dev, &it->bridge);
>>>   }
>>>
>>
>>
> 
> 


-- 
Regards,
Liu Ying
Re: [PATCH 3/7] drm/bridge: ite-it6232: declare supported infoframes
Posted by Dmitry Baryshkov 1 month, 2 weeks ago
On 20/08/2025 06:28, Liu Ying wrote:
> On 08/19/2025, Dmitry Baryshkov wrote:
>> On 19/08/2025 12:49, Liu Ying wrote:
>>> Hi Dmitry,
>>>
>>> On 08/16/2025, Dmitry Baryshkov wrote:
>>>> Declare which infoframes are supported via the .hdmi_write_infoframe()
>>>> interface.
>>>>
>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>>>> ---
>>>>    drivers/gpu/drm/bridge/ite-it6263.c | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>
>>> For subject, s/it6232/it6263.
>>
>> Ack
>>
>>>
>>>>
>>>> diff --git a/drivers/gpu/drm/bridge/ite-it6263.c b/drivers/gpu/drm/bridge/ite-it6263.c
>>>> index cf813672b4ffb8ab5c524c6414ee7b414cebc018..b1956891a8388401c13cd2fc5c78f0779063adf4 100644
>>>> --- a/drivers/gpu/drm/bridge/ite-it6263.c
>>>> +++ b/drivers/gpu/drm/bridge/ite-it6263.c
>>>> @@ -875,6 +875,7 @@ static int it6263_probe(struct i2c_client *client)
>>>>        it->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
>>>>        it->bridge.vendor = "ITE";
>>>>        it->bridge.product = "IT6263";
>>>> +    it->bridge.supported_infoframes = DRM_CONNECTOR_INFOFRAME_AVI;
>>>
>>> With supported_infoframes set, the two lines setting vendor and product
>>> are dead code.  I think it's worth dropping them and updating kerneldoc
>>> for vendor and product members because they don't have to be required if
>>> DRM_BRIDGE_OP_HDMI is set.  But, this could be done with future patch(not
>>> in this patch series).
>>
>> They are still required by the framework itself, see drmm_connector_hdmi_init().
> 
> Yes.  But it's a bit too strict since SPD infoframe is optional according
> to CTA standard documentation.
> 
>>
>> BTW: I don't have ITE datasheet. Do you know if it really supports only the AVI frame?
> 
> AFAICS, it seems that ITE6263 supports inforframes from 0x81 to 0x85, so
> including SPD inforframe.  Maybe, just keep those dead vendor&product
> settings for now and add SPD inforframe in future.

I don't have necessary hardware (nor programming manual). If you can 
extend support for those frames, a patch would be really appreciated.

> 
>>
>>
>>>
>>> Reviewed-by: Liu Ying <victor.liu@nxp.com>
>>>
>>>>          return devm_drm_bridge_add(dev, &it->bridge);
>>>>    }
>>>>
>>>
>>>
>>
>>
> 
> 


-- 
With best wishes
Dmitry