On 11/23/22 16:25, Maxime Ripard wrote:

On the vc4 devices (and later), the blending is done by a single device called the HVS. The HVS has three FIFO that can operate in parallel, and route their output to 6 CRTCs and 7 encoders on the BCM2711.

Each of these CRTCs and encoders have some contraints on which FIFO they

constraints.

can feed from, so we need some code to take all those constraints into account and assign FIFOs to CRTCs.

The problem can be simplified by assigning those FIFOs to CRTCs by ascending output index number. We had a comment mentioning it already, but we were never actually enforcing it.

It was working still in most situations because the probe order is roughly equivalent, except for the (optional, and fairly rarely used on the Pi4) VEC which was last in the probe order sequence, but one of the earliest device to assign.

This resulted in configurations that were rejected by our code but were still valid with a different assignment.

We can fix this by making sure we assign CRTCs to FIFOs by ordering them by ascending HVS output index.

Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically") Signed-off-by: Maxime Ripard maxime@cerno.tech

---

[...]

• for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {

• struct vc4_crtc_state *old_vc4_crtc_state =
  

• 	to_vc4_crtc_state(old_crtc_state);
  

• struct vc4_crtc_state *new_vc4_crtc_state =
  

• 	to_vc4_crtc_state(new_crtc_state);
  

• struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
  

• /*

• * The problem we have to solve here is that we have up to 7
  

• * encoders, connected to up to 6 CRTCs.
  

• *
  

• * Those CRTCs, depending on the instance, can be routed to 1, 2
  

• * or 3 HVS FIFOs, and we need to set the change the muxing
  

This sentence sounds a little bit off to me. Did you mean:

"we need to set the muxing between" or "we need to change the muxing" ?

I'm not familiar with VC4 but the patch seems to do what the commit message says, so the changes look good to me.

Reviewed-by: Javier Martinez Canillas javierm@redhat.com