Re: [PATCH v5 80/80] ARM: dts: bcm2711: Enable the display pipeline

[Nicolas Saenz Julienne]

Hi Dave, sorry for the late reply.

On Tue, 2020-10-06 at 18:14 +0100, Dave Stevenson wrote:
> Hi Maxime
> 
> On Tue, 6 Oct 2020 at 16:26, Maxime Ripard <maxime@xxxxxxxxxx> wrote:
> > Hi Dave,
> > 
> > On Fri, Oct 02, 2020 at 04:57:05PM +0100, Dave Stevenson wrote:
> > > Hi Maxime
> > > 
> > > On Fri, 2 Oct 2020 at 16:19, Maxime Ripard <maxime@xxxxxxxxxx> wrote:
> > > > Hi Tim,
> > > > 
> > > > On Thu, Oct 01, 2020 at 11:15:46AM +0100, Tim Gover wrote:
> > > > > hdmi_enable_4k60=1 causes the firmware to select 3.3 GHz for the PLLC
> > > > > VCO to support a core-frequency of 550 MHz which is the minimum
> > > > > frequency required by the HVS at 4Kp60. The side effect is that if the
> > > > > display clock requirements are lower than 4Kp60 then you will see
> > > > > different core frequencies selected by DVFS.
> > > > > 
> > > > > If enable_uart=1 and the mini-uart is selected (default unless
> > > > > bluetooth is disabled) then the firmware will pin the core-frequency
> > > > > to either core_freq max (500 or 550). Although, I think there is a way
> > > > > of pinning it to a lower fixed frequency.
> > > > > 
> > > > > The table in overclocking.md defines options for setting the maximum
> > > > > core frequency but unless core_freq_min is specified DVFS will
> > > > > automatically pick the lowest idle frequency required by the display
> > > > > resolution.
> > > > 
> > > > I'm wondering if there's some way to detect this from Linux? I guess it
> > > > would be nice to be able to at least detect a broken config to warn /
> > > > prevent an user that their situation is not going to be reliable / work
> > > > really well (like if they have a 4k display without hdmi_enable_4kp60
> > > > set, or the issue we're discussing here)
> > > 
> > > The main filter in the firmware is the parameter
> > > hdmi_pixel_freq_limit. That can either be set manually from
> > > config.txt, or defaults appropriately based on hdmi_enable_4kp60.
> > > Under firmware_kms [1] I read back those values to use as a filter
> > > within crtc_mode_valid[2].
> > > I can't think of a nice way of exposing that without the vc4 driver
> > > gaining a DT link to the firmware, and that starts to get ugly.
> > 
> > I had in mind something like if the clock driver can infer that somehow
> > through some the boundaries reported by the firmware maybe? IIRC,
> > hdmi_enable_4kp60 will already change the max frequency reported to
> > 550MHz instead of 500MHz
> 
> Yes, that's plausible, but I don't know enough about the clock
> infrastructure for advertising limits to know what works there.
> Tell me what you need from the mailbox service and I'll see what I can do.
> 
> We do already have RPI_FIRMWARE_GET_MAX_CLOCK_RATE and
> RPI_FIRMWARE_GET_MIN_CLOCK_RATE. It'd take a few minutes of staring at
> the code (or a quick test) to confirm if they definitely are changed
> for CORE clock by hdmi_enable_4kp60 - I think it does.

Tim commented earlier that you meant to pin CORE frequency when enable_uart=1.
In practice it doesn't seem to be the case, but it would solve the UART
problem for most use cases. Would that be feasible?

If we have to change the CORE frequency during runtime we could, while we
search for a proper solution, print a warning that we're about to break the low
speed peripherals.

Regards,
Nicolas

Attachment: signature.asc
Description: This is a digitally signed message part