Re: [PATCH v2 2/8] drm/vc4: hdmi: Bump BCM2835 max pixel clock to 165 MHz

[Cristian Ciocaltea]

Hi Dave,

On 10/13/26 6:59 PM, Dave Stevenson wrote:
> Hi Cristian
> 
> On Tue, 13 Oct 2026 at 01:16, Cristian Ciocaltea
> <cristian.ciocaltea@xxxxxxxxxxxxx> wrote:
>>
>> Hi Dave,
>>
>> On 10/13/26 1:37 AM, Cristian Ciocaltea wrote:
>>> Historically, the HSM clock for BCM2835 was fixed at 163.7 MHz.  Since
>>> the pixel clock had to be at least 1% below the HSM clock, it was capped
>>> at 162 MHz.
>>>
>>> Since commit cd4cb49dc5bb ("drm/vc4: hdmi: Adjust HSM clock rate
>>> depending on pixel rate"), the HSM clock rate is set to pixel clock *
>>> 101 / 100.  For a 165 MHz pixel clock, this gives an HSM rate of 166.65
>>> MHz, which, per Dave Stevenson, the hardware can handle without issues.
>>>
>>> Increase the max pixel clock to 165 MHz, which matches the maximum TMDS
>>> character rate of HDMI 1.0 through 1.2.
>>
>> It seems this will cause CEC register corruption:
>>
>> https://sashiko.dev/#/patchset/20261013-vc4-common-scramb-v2-0-a41d15e131ad@xxxxxxxxxxxxx?part=2
>>
>> We can easily address this by clamping the CEC clock divider to its register
>> field width:
>>
>> -       clk_cnt = cec_rate / CEC_CLOCK_FREQ;
>> +       clk_cnt = min_t(unsigned long, cec_rate / CEC_CLOCK_FREQ,
>> +                       VC4_HDMI_CEC_DIV_CLK_CNT_MASK >>
>> +                       VC4_HDMI_CEC_DIV_CLK_CNT_SHIFT);
>>
>> The consequence is that the CEC clock would run slightly faster than the nominal
>> 40 kHz, i.e. (165000 × 101/100) / (2^12 - 1) = 40.6959 kHz.
>>
>> I think this is still within the CEC bit-timing tolerances and shouldn't cause
>> any issues.  Could you please confirm?
> 
> My reading of the CEC spec agrees.
> 
> All the timings have tolerance of +/- 0.2ms on values up to 4.5ms, so
> that would allow up to a 4.3% tolerance on frequency.
> The error from clipping the clock divider is 1.74%, so fairly
> comfortably within tolerance.
> 
> It's only video modes with clocks in the range 162-165MHz that will
> have any inaccuracy, so pretty limited in scope anyway.

Yep, the clamp would trigger only for pixel clocks above 162.178 MHz.

> Working backwards, achieving a 40kHz output with the max divider works
> out to be a 163.8MHz HSM clock, so that would appear to be why the
> limit was introduced in the first place.

Quite possible, indeed.

So I'll prepare v3, which adds yet another prerequisite patch to clamp the CEC
clock divider.

Thanks,
Cristian