vc4/drm: Remove the clear of SCALER_DISPBKGND_FILL

[popcornmix]

Since "drm/vc4: hvs: Support BCM2712 HVS" booting Pi4 with dual 4kp30 displays connected fails with:
vc4-drm gpu: [drm] ERROR [CRTC:107:pixelvalve-4] flip_done timed out

It has been tracked down to the referenced commit adding a path to clear the SCALER_DISPBKGND_FILL when not required.

While that appears valid, it causes a problem
(that still needs to be understood).

Lets revert that change to avoid the regression for now.

Fixes: e84da23 ("drm/vc4: hvs: Support BCM2712 HVS")

[popcornmix]

From spec:

Background Fill Mode
1 = Enable background fill.
0 = Disable background fill.
When background fill is enabled each output line in the FIFO will be pre-filled with the background colour prior to compositing but at some cost in available scaler engine cycles. If background fill is not enabled then the colour of pixels not involved in a compositing operating will be undefined but if an opaque image is used to fill the whole display (eg. a video stream) then there will be no un-composited pixels so the saving in scaler engine cycles is useful.

If I run an old kernel (prior to the "drm/vc4: hvs: Support BCM2712 HVS" commit) with dual 4kp30, and then poke:
DISPBKGND0:BGENB=0
both displays become unreliable and alternate between working image and no signal.

Setting:
DISPBKGND0:BGENB=1
makes the two displays reliable again.

Dual 4kp30 results in a core freq min request of 297MHz.
Adding, say core_freq_min=400 avoids the blanking issue with DISPBKGND0:BGENB=0.

My guess is that clearing the background, whilst consuming HVS cycles is beneficial in allowing a low core clock, perhaps by spreading out sdram requests at the start of a line/frame.

[popcornmix]

@pelwell @6by9 any comments? Would be good to get this into tonight's nightly.

[6by9]

Seems reasonable for now until we understand it better.

[pelwell]

It obviously seems to be working for you, but is it always going to be sufficient to not clear the background fill enable bit? I don't like the asymmetry in the code paths (enable_bg_fill -> set the bit, !enable_bg_fill -> do nothing), and not from an OCD perspective.

[pelwell]

( I don't mind if you want to merge for the nightly )

[6by9]

Always doing the background fill consumes a few more cycles in the HVS, but won't have any visible effects.

It does look like the old code path did leave it as a little bit of a sticky flag - once set due to being needed once, it never got cleared.

Longer term I'd be tempted to rearrange it as

if (vc4->gen >= VC4_GEN_6) {
   if (enable_bg_fill)
      //set HVS6
    else
       // clear HVS6
} else {
  //Comment as to why
  //set HVS4/5
}

but I don't see that as needed for now.

[popcornmix]

I'll switch to @6by9's suggestion.

[popcornmix]

So:
core=297MHz is okay when setting background fill
core=320MHz fails with background fill cleared
core=330MHz is okay with background fill cleared

so it's requiring a significantly higher clock if you don't set background fill.

[popcornmix]

Updated PR to match @6by9 's layout.

I agree with @pelwell that the symmetry of always enabling background fill is tempting,
but given all Pi5 testing has been with the clear in place and Pi0-4 previously always has it set,
it's safer to stick with tested settings for the stable kernel.

Perhaps we'll switch to the symmetric option for the newer kernel trees for further testing.