[Intel-gfx] [PATCH] drm/i915: Treat resetting of the current framebuffer as a no-op

[Daniel Vetter]

On Fri, May 31, 2013 at 7:50 PM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> On Fri, May 31, 2013 at 05:15:10PM +0200, Daniel Vetter wrote:
>> On Fri, May 31, 2013 at 4:05 PM, Paulo Zanoni <przanoni at gmail.com> wrote:
>> > 2013/5/23 Daniel Vetter <daniel at ffwll.ch>:
>> >> On Thu, May 23, 2013 at 01:57:17PM +0100, Chris Wilson wrote:
>> >>> If none of the CRTC parameters change along with the framebuffer, we can
>> >>> forgo rewriting the register and waiting for a vblank. There are a few
>> >>> calls made by the display managers as they start up which tend to end up
>> >>> performing no-ops on the current CRTC settings.
>> >>>
>> >>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>> >>
>> >> Makes sense. Queued for -next, thanks for the patch. Now the only things
>> >> left (besides beating fastboot into good shape) is to cache the edids a
>> >> bit and we've (hopefully) killed all kms stalls at startup ...
>> >
>> > This commit introduced a regression.
>> >
>> > - Boot with both eDP and DP plugged
>> > - When I boot like this, eDP1 has 1920x1080 and DP1 has 1920x1080i.
>> > - Run "xrandr --output DP1 --mode 0x55" (that's 1024x768 at 60Hz here)
>> > - See the black screen on DP output, dmesg has the "skipping reset of
>> > current fb" message.
>> > - After we get the black screen, if we run "xrandr --output DP1 --off;
>> > xrandr --output DP1 --mode 0x55" the mode will work.
>> >
>> > If I diff the "bad state" with the "good state" we'll see the cause is
>> > the DSPCNTR register. When we do the early return in
>> > intel_pipe_set_base we don't call the update_plane function. For me
>> > what changes is the pixel format and the trickle feed bits.
>>
>> Oh, in the modeset case we can't optimize the update_fb away, even
>> when both fbs are the same ...
>
> Hopefully we can push the check higher to the fb fast path then? The
> skip is much safer in the kernel as it has correct knowledge of the
> current state (as opposed to trying to track it the ddx sharing control
> of the machine).

Yeah, I think we could move it out into the setcrtc logic. Dropped the
patch in it's current version for now.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch