Re: [Freedreno] [PATCH RFC v6 07/10] drm/atomic: Loosen FB atomic checks

[Jessica Zhang]

On 8/29/2023 1:22 AM, Pekka Paalanen wrote:
> On Mon, 28 Aug 2023 17:05:13 -0700
> Jessica Zhang <quic_jesszhan@xxxxxxxxxxx> wrote:
>
> > Loosen the requirements for atomic and legacy commit so that, in cases
> > where pixel_source != FB, the commit can still go through.
> >
> > This includes adding framebuffer NULL checks in other areas to account for
> > FB being NULL when non-FB pixel sources are enabled.
> >
> > To disable a plane, the pixel_source must be NONE or the FB must be NULL
> > if pixel_source == FB.
> >
> > Signed-off-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>
> > ---
> >   drivers/gpu/drm/drm_atomic.c        | 20 +++++++++++---------
> >   drivers/gpu/drm/drm_atomic_helper.c | 36 ++++++++++++++++++++----------------
> >   include/drm/drm_atomic_helper.h     |  4 ++--
> >   include/drm/drm_plane.h             | 29 +++++++++++++++++++++++++++++
> >   4 files changed, 62 insertions(+), 27 deletions(-)
>
> ...
>
> > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> > index a58f84b6bd5e..4c5b7bcdb25c 100644
> > --- a/include/drm/drm_plane.h
> > +++ b/include/drm/drm_plane.h
> > @@ -992,6 +992,35 @@ static inline struct drm_plane *drm_plane_find(struct drm_device *dev,
> >   #define drm_for_each_plane(plane, dev) \
> >   	list_for_each_entry(plane, &(dev)->mode_config.plane_list, head)
> >   
> > +/**
> > + * drm_plane_solid_fill_enabled - Check if solid fill is enabled on plane
> > + * @state: plane state
> > + *
> > + * Returns:
> > + * Whether the plane has been assigned a solid_fill_blob
> > + */
> > +static inline bool drm_plane_solid_fill_enabled(struct drm_plane_state *state)
> > +{
> > +	if (!state)
> > +		return false;
> > +	return state->pixel_source == DRM_PLANE_PIXEL_SOURCE_SOLID_FILL && state->solid_fill_blob;
> > +}
> > +
> > +static inline bool drm_plane_has_visible_data(const struct drm_plane_state *state)
> > +{
> > +	switch (state->pixel_source) {
> > +	case DRM_PLANE_PIXEL_SOURCE_NONE:
> > +		return false;
> > +	case DRM_PLANE_PIXEL_SOURCE_SOLID_FILL:
> > +		return state->solid_fill_blob != NULL;
>
> This reminds me, new UAPI docs did not say what the requirements are for
> choosing solid fill pixel source. Is the atomic commit rejected if
> pixel source is solid fill, but solid_fill property has no blob?

Hi Pekka,

Yes, if pixel_source is solid_fill and the solid_fill property blob 
isn't set, the atomic commit should throw an error.

Will document this in the UAPI.

Thanks,

Jessica Zhang

> This should be doc'd.
>
>
> Thanks,
> pq
>
> > +	case DRM_PLANE_PIXEL_SOURCE_FB:
> > +	default:
> > +		WARN_ON(state->pixel_source != DRM_PLANE_PIXEL_SOURCE_FB);
> > +	}
> > +
> > +	return state->fb != NULL;
> > +}
> > +
> >   bool drm_any_plane_has_format(struct drm_device *dev,
> >   			      u32 format, u64 modifier);
> >