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

[Daniel Vetter]

On Fri, May 24, 2013 at 7:06 PM, Ville Syrjälä
<ville.syrjala at linux.intel.com> wrote:
> On Fri, May 24, 2013 at 05:14:40PM +0200, Daniel Vetter wrote:
>> 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?
>
> I was mainly thinking about the current drivers that already depend on
> planes for scanout. We don't want to go blindly disable all planes in
> the core since the driver is the one that knows which plane it needs for
> the crtc scanout duties.

I guess those currently have a plane reserved for each crtc (or
otherwise need to be careful not to glober crtc state if the implicit
crtc plane is used as a real kms plane). I guess we want to eventually
make that implicit plane explicit, but I guess that can only be done
with a feature flag. At least I don't see any way to do this without
breaking the established kms abi.

Maybe Rob could chime in?

[snip]

>> > 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.
>
> So what about set_par/pan_display? Do you want the fbdev restore thingy
> to permanently disable the planes/cursors, as in update our s/w state to
> indicate that they're disabled and hence won't get restored unless
> someone explicitly re-enables them? Because that's not what the current
> code does.

Yeah, I think it should just clobber the kms state. Clients will
restore everything when vt-switching anyway and if we oops it doesn't
matter that much since the system is going down the toilet anyway.

That leaves kgdb, but since kgdb is currently broken when the active
client uses a sprite (which might cover up kgdb) I don't think it's
worse if it's broken in a different fashion afterwards (by killing the
sprite state once the client will be restored. And we should be able
to fix this by storing the sprite state somewhere ...

Maybe we need to add a better getfoo ioctl once we have atomic modeset
to better support a system compositor. That way it could reconstruct
the kms state and flip clients in userspace.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch