Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> writes:
> On Tue, Mar 06, 2018 at 04:10:33PM -0800, Eric Anholt wrote:
>> Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> 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@xxxxxxxxx>
>> >> ---
>> >> 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.
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
