[PATCH driver/intel] Allow copy_front() to fail and clean up gracefully if it does

[Chris Wilson]

On Fri, Sep 26, 2014 at 06:05:07PM +0200, Egbert Eich wrote:
> 
> Hi Chris,
> 
> Chris Wilson writes:
>  > 
>  > Also realised that even this should be fixed up as a last resort by
>  > falling back to the shadow CRTC allocation. That band-aid should be
>  > working again.
>  > 
>  > Hopefully
>  > 
>  > commit 9f7c1a4c4f2a6352263c36e75a984ed4095adbc0
>  > Author: Chris Wilson <chris at chris-wilson.co.uk>
>  > Date:   Thu Sep 25 16:29:14 2014 +0100
>  > 
>  >     sna: Check for scanout pitch restrictions on linear GPU bo
>  >     
>  >     When converting a linear cached CPU bo into an uncached GPU bo, we must
>  >     be careful to adhere to the scanout restrictions if they apply for this
>  >     transfer or this Pixmap.
>  >     
>  >     Reported-by: Egbert Eich <eich at suse.com>
>  >     Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>  > 
>  > catches all the cases where we need to check the alignment on the pitch
>  > before changing the cache level. If not, it now explicitly converts the
>  > bo before making the framebuffer.
> 
> thanks for looking into this! I can confirm that the patch works.
> 
> I took a brief look into why the server terminates at server reset
> when the the patch which fixes the tiling isn't applied. 
> This happens in sna_create_screen_resources() when sna_pixmap_force_to_gpu() 
> fails and the entire creatSecreenResources fails.
> Since sna_pixmap_force_to_gpu() is allowed to fail at other places
> as alternatives will be used I looked who might need this call. 
> The only thing I found was sna_copy_fbcon() which will fail in 
> assert(priv && priv->gpu_bo). sna_copy_fbcon() is however not vital:
> The fix below made this work for me.
> This is a simple hack, one could also use a bo to a supported front 
> buffer instead of insisiting a GPU bo.
> 
> Later on use_shadow() should take care of a the failure of 
> sna_pixmap_force_to_gpu() in sna_create_screen_resources(). 
> 
> Or can you think of another reason why this is needed so early?

The move-to-gpu exists so that we initialise the GPU bo early on so that
we didn't incur any delay when the time came to display the framebuffer.
(It also helped making the headless case similar to normal setups for
testing purposes.)

I thought making it fatal would help detect configuration errors easier
- but indeed the modeset has a good chance of recovering from the error.

I am convinced (at least by 3/4 of that patch, I still want the
assert(priv)!)

Thanks,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre