Re: [PATCH v4 10/22] drm: omapdrm: Let the DRM core skip plane commit on inactive CRTCs

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Tomi,

On Wednesday 14 Dec 2016 12:43:54 Tomi Valkeinen wrote:
> On 14/12/16 02:27, Laurent Pinchart wrote:
> > The DRM core supports skipping plane update for inactive CRTCs for
> > hardware that don't need it or can't cope with it. That's our case, and
> > the driver already skips flushing planes on inactice CRTCs.
> > 
> > We can't remove the check from the driver, as active CRTCs are disabled
> > at the hardware level when an atomic flush is performed if a mode set is
> > pending. There's however no need to forward the plane commit calls to
> > the driver, so use the DRM core infrastructure to skip them with a
> > detailed comment to explain why the check must still be kept in the
> > driver.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > Changes since v3:
> > 
> > - Reworded commit message
> > ---
> > 
> >  drivers/gpu/drm/omapdrm/omap_crtc.c | 8 +++++++-
> >  drivers/gpu/drm/omapdrm/omap_drv.c  | 3 ++-
> >  2 files changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> > b/drivers/gpu/drm/omapdrm/omap_crtc.c index 42c3b44f9689..2832dbffd873
> > 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> > @@ -414,7 +414,13 @@ static void omap_crtc_atomic_flush(struct drm_crtc
> > *crtc,> 
> >  		dispc_mgr_set_gamma(omap_crtc->channel, lut, length);
> >  	}
> > 
> > -	/* Only flush the CRTC if it is currently enabled. */
> > +	/*
> > +	 * Only flush the CRTC if it is currently enabled. CRTCs that require
> > a
> > +	 * mode set are disabled prior plane updates and enabled afterwards.
> > +	 * They are thus not active (regardless of what their CRTC core state
> > +	 * reports) and the DRM core could thus call this function even though
> > +	 * the CRTC is currently disabled. Do nothing in that case.
> > +	 */
> >  	if (!omap_crtc->enabled)
> >  		return;
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> > b/drivers/gpu/drm/omapdrm/omap_drv.c index 6faba13c8e41..0a2d461d62cf
> > 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> > @@ -96,7 +96,8 @@ static void omap_atomic_complete(struct
> > omap_atomic_state_commit *commit)
> >  	dispc_runtime_get();
> >  	
> >  	drm_atomic_helper_commit_modeset_disables(dev, old_state);
> > -	drm_atomic_helper_commit_planes(dev, old_state, 0);
> > +	drm_atomic_helper_commit_planes(dev, old_state,
> > +					DRM_PLANE_COMMIT_ACTIVE_ONLY);
> >  	drm_atomic_helper_commit_modeset_enables(dev, old_state);
> 
> Just to make sure. "ACTIVE" in this context means a crtc that's already
> enabled, or will be enabled at the
> drm_atomic_helper_commit_modeset_enables() call?

Correct, it covers both.

> If so:
> 
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel