[PATCH 2/3] drm/vc4: Check if plane requires background fill

[Eric Anholt]

Ville Syrjälä <ville.syrjala at linux.intel.com> writes:

> On Tue, Mar 06, 2018 at 04:10:33PM -0800, Eric Anholt wrote:
>> Ville Syrjälä <ville.syrjala at linux.intel.com> writes:
>> 
>> > On Tue, Mar 06, 2018 at 02:48:38AM +0100, Stefan Schake wrote:
>> >> Considering a single plane only, we have to enable background color
>> >> when the plane has an alpha format and could be blending from the
>> >> background or when it doesn't cover the entire screen.
>> >> 
>> >> Signed-off-by: Stefan Schake <stschake at gmail.com>
>> >> ---
>> >>  drivers/gpu/drm/vc4/vc4_drv.h   |  6 ++++++
>> >>  drivers/gpu/drm/vc4/vc4_plane.c | 15 ++++++++++++++-
>> >>  2 files changed, 20 insertions(+), 1 deletion(-)
>> >> 
>> >> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
>> >> index fefa166..7cc6390 100644
>> >> --- a/drivers/gpu/drm/vc4/vc4_drv.h
>> >> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
>> >> @@ -302,6 +302,12 @@ struct vc4_hvs {
>> >>  
>> >>  struct vc4_plane {
>> >>  	struct drm_plane base;
>> >> +
>> >> +	/* Set when the plane has per-pixel alpha content or does not cover
>> >> +	 * the entire screen. This is a hint to the CRTC that it might need
>> >> +	 * to enable background color fill.
>> >> +	 */
>> >> +	bool needs_bg_fill;
>> >
>> > Looks to me like that should really be a bitmask (or something similar)
>> > in the crtc state.
>> 
>> Why?
>> 
>> In particular, VC4 really doesn't have a fixed number of planes, and the
>> fact that we're exposing just a handful so far is probably a bug.
>
> The problem is that this seems to clobber the device state from the
> .atomic_check() hook. So if you do a CHECK_ONLY atomic ioctl (or
> some later check simply fails and the operation is aborted) you've
> already modified the state of the device, and some later operation
> may then end up doing the wrong thing.
>
> I guess you could track this in the plane struct like here, but as
> with the actual hardware state that shouldn't get modified until
> we're sure the checked state is really meant to be commited to the
> hardware.

Oh, I hadn't noticed it was in vc4_plane, not vc4_plane_state.  Yeah, it
should be in the plane state.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-rpi-kernel/attachments/20180307/f1997e32/attachment.sig>