[Intel-gfx] [PATCH 3/7] drm/i915: Fix fbdev sprite disable code

[Daniel Vetter]

On Fri, May 24, 2013 at 11:59 AM, Ville Syrjälä
<ville.syrjala at linux.intel.com> wrote:
>
>> Why exactly do we need special driver callbacks for all this? Strictly
>> speaking fbcon is just another kms client, if it needs to dig around in
>> driver details that just means our interfaces are not good enough. I know
>> that there's some historical baggage left around, but for disabling
>> cursors and planes I don't see any need at all:
>>
>> Can't we just loop over all crtcs and disable every cursor and then loop
>> over all sprites and also shut them all down when restoring the fbdev
>> mode? All without any new driver callbacks?
>
> Some driver might want to use hardware cursor for fb_cursor, also some
> drivers want to use planes for fbdev scanout, which as you know is
> something I want for i915 as well in the future.

I guess the aim is to use the plane scaler and blast the fbcon output
over whatever resolution we're currently displaying for e.g. an oops?
Even then I don't see how this is any different from Wayland blasting
a fullscreen app all over the screen. And if our drm core -> driver
interface isn't good enough for fbcon to do that, Wayland won't ever
get it right.

So all I'm saying is that proper fbcon safe/restore looks like a neat
in-kernel test-bed to check interfaces. And we should go with that
approach instead of rolling driver hacks. And splattering
is_fbcon_crtc checks all over _is_ imo a great hack.

> Also there's the whole state mismanagement. We don't track enough of
> the state in the core to restore stuff properly, so if we disable
> stuff, we can't re-enable it when switching back to the client using
> sprites/planes. We could move the plane coordinate tracking to the
> core easily enough (I even had a patch for that in some old atomic
> branch). I haven't looked at out current cursor stuff closely enough
> to figure out if we can do the same there, but currently the cursor
> state is tracked purely inside the driver.

Atm kms client state management is purely driven by vt switching, and
we don't have any guarantees at all that state survives a vt switch.
Hence both X and Wayland already fully restore kms state. So in the
current model we don't need any state safe/restore in the kernel.

Also I'm not sure whether doing that state safe/restore in the kernel
is the right approach to handle switching between multiple
compositors. I expect that we want a system compositor (for nice
transitions) and then probably also we need a new ownership model to
ensure that compositors don't have access to each anothers stuff.

> So for the short term I think it's easier to go with the new hook,
> and let each driver manage their planes/cursors.

Since we can't avoid the flickering right now (at least in the
set_base fastpath, which should be common) the only thing we need is
to loop over all cursors and sprites in fbdev_restore_mode and kill
them. X/Wayland will restore already. That's one small patch afaics
and so imo the right solution short-term.

And long-term we'd have a simple testcase for the atomic modeset
interfaces, which might help a bit with the dri-devel bikeshedding.

>> It might flicker like mad since we can't do it atomically, but then again
>> this would give us a neat in-kernel user for the atomic modeset stuff
>> right away ...
>
> When we get to the atomic stuff, I'd like to keep a full state per
> client, and then it'd be very easy to just blast the new state in when
> swithing between clients.

See above, I don't think we need that. Or at least there's a lot of
open questions around it imo.

Generally though I really think we should move away from driver
interfaces for fbcon. Currently we have
- fb allocation - I think that could be done with the dumb ioctl
interface instead, or at least it should be possible to do so. Tbh
haven't looked into it.
- initial mode selection. Can imo only be fixed for real with the
check-only mode of the atomic modeset. Till then we have a bit an
abstraction leak.
- gamma settings. Pure duplication with the kms interfaces, is on my
list of things to kill.

The _only_ places fbdev support should need driver hooks are imo are
if a driver wants extended fbdev emulation (e.g. render accel), and
for those cases it can simply overwrite the fbdev vtable with its own
functions (which might or might not call down into the fbdev helper).

But for basic modeset operations there's imo really no need for
special fbdev helper interfaces, our core drm -> driver interface
should be good enough.

Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch