linux-kernel - Re: [PATCH] drm/atomic: Handling the case when setting old crtc for plane

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180503105503.GQ23723@intel.com>
Date:   Thu, 3 May 2018 13:55:03 +0300
From:   Ville Syrjälä <ville.syrjala@...ux.intel.com>
To:     Satendra Singh Thakur <satendra.t@...sung.com>,
        Gustavo Padovan <gustavo@...ovan.org>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Sean Paul <seanpaul@...omium.org>,
        David Airlie <airlied@...ux.ie>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        Hemanshu Srivastava <hemanshu.s@...sung.com>,
        Madhur Verma <madhur.verma@...sung.com>
Subject: Re: [PATCH] drm/atomic: Handling the case when setting old crtc for
 plane

On Thu, May 03, 2018 at 11:24:59AM +0200, Daniel Vetter wrote:
> On Thu, May 03, 2018 at 11:19:32AM +0530, Satendra Singh Thakur wrote:
> > In the func drm_atomic_set_crtc_for_plane, with the current code,
> > if crtc of the plane_state and crtc passed as argument to the func
> > are same, entire func will executed in vein.
> > It will get state of crtc and clear and set the bits in plane_mask.
> > All these steps are not required for same old crtc.
> > Ideally, we should do nothing in this case, this patch handles the same,
> > and causes the program to return without doing anything in such scenario.
> > 
> > Signed-off-by: Satendra Singh Thakur <satendra.t@...sung.com>
> > Cc: Madhur Verma <madhur.verma@...sung.com>
> > Cc: Hemanshu Srivastava <hemanshu.s@...sung.com>
> > ---
> >  drivers/gpu/drm/drm_atomic.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 7d25c42..5bd3365 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -1421,7 +1421,9 @@ drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state,
> >  {
> >  	struct drm_plane *plane = plane_state->plane;
> >  	struct drm_crtc_state *crtc_state;
> > -
> > +	/* Nothing to do for same crtc*/
> > +	if (plane_state->crtc == crtc)
> > +		return 0;
> 
> I didn't do this (both here and in the set_crtc_for_connector functions)
> because the overhead is probably way down in the noise compared to the
> overall atomic commit machinery. Do you really see this as a hotpath?

drm_atomic_set_crtc_for_connector() actually has this since commit
e2d800a3ce1b ("drm: Avoid connector reference imbalance on error path")

-- 
Ville Syrjälä
Intel