Re: [PATCH v2 01/14] phy: hdmi: Add HDMI 2.1 FRL configuration options

[Cristian Ciocaltea <cristian.ciocaltea@collabora.com>]

Hi Dmitry,

On 8/16/25 1:23 AM, Dmitry Baryshkov wrote:
> On 05/08/2025 14:56, Cristian Ciocaltea wrote:
>> Add support for configuring the Fixed Rate Link (FRL) mode for HDMI
>> PHYs.
>>
>> For improved clarity, also rename existing HDMI mode to HDMI_TMDS.
>> Since this mode is currently not being used explicitly, no further
>> changes are needed.
> 
> 
> I'd say, this need some explanation, especially for those who don't have access 
> to the HDMI standard. Please mention that FRL is an alternative to the traditional TMDS mode, etc.

Done in v3 [1].

> 
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>   include/linux/phy/phy-hdmi.h | 14 ++++++++++++--
>>   include/linux/phy/phy.h      |  3 ++-
>>   2 files changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/phy/phy-hdmi.h b/include/linux/phy/phy-hdmi.h
>> index f0ec963c6e84f1b7728acafc824dff191c6b873d..0b26472d8defcc9ded142d4283e29861dc2b3746 100644
>> --- a/include/linux/phy/phy-hdmi.h
>> +++ b/include/linux/phy/phy-hdmi.h
>> @@ -6,16 +6,26 @@
>>   #ifndef __PHY_HDMI_H_
>>   #define __PHY_HDMI_H_
>>   +#include <linux/types.h>
>> +
>>   /**
>>    * struct phy_configure_opts_hdmi - HDMI configuration set
>> - * @tmds_char_rate: HDMI TMDS Character Rate in Hertz.
>>    * @bpc: Bits per color channel.
>> + * @tmds_char_rate: HDMI TMDS Character Rate in Hertz.
>> + * @frl.rate_per_lane: HDMI FRL Rate per Lane in Gbps.
>> + * @frl.lanes: HDMI FRL lanes count.
>>    *
>>    * This structure is used to represent the configuration state of a HDMI phy.
>>    */
>>   struct phy_configure_opts_hdmi {
>> -    unsigned long long tmds_char_rate;
>>       unsigned int bpc;
>> +    union {
>> +        unsigned long long tmds_char_rate;
>> +        struct {
>> +            u8 rate_per_lane;
>> +            u8 lanes;
>> +        } frl;
>> +    };
>>   };
>>     #endif /* __PHY_HDMI_H_ */
>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
>> index 13add0c2c40721fe9ca3f0350d13c035cd25af45..04c84c1dbd6dac55fd04c54203cff4f9d939d970 100644
>> --- a/include/linux/phy/phy.h
>> +++ b/include/linux/phy/phy.h
>> @@ -44,7 +44,8 @@ enum phy_mode {
>>       PHY_MODE_SATA,
>>       PHY_MODE_LVDS,
>>       PHY_MODE_DP,
>> -    PHY_MODE_HDMI,
>> +    PHY_MODE_HDMI_TMDS,
>> +    PHY_MODE_HDMI_FRL,
> 
> I'd maybe suggest keeping PHY_MODE_HDMI and having two submodes: one for TMDS (default), one for FRL.

Also done in v3 [1].

Thank you for the review!

Regards,
Cristian

> 
>>   };
>>     enum phy_media {
>>

[1] https://lore.kernel.org/all/20250818-phy-hdptx-frl-v3-0-c79997d8bb2b@collabora.com/