Re: [PATCH 4/5] drm/atomic: document how to handle driver private objects

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Alex Deucher <alexdeucher@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	DRI Development <dri-devel@lists.freedesktop.org>,
	Ben Skeggs <bskeggs@redhat.com>,
	Alex Deucher <alexander.deucher@amd.com>,
	Daniel Vetter <daniel.vetter@intel.com>,
	Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Subject: Re: [PATCH 4/5] drm/atomic: document how to handle driver private objects
Date: Mon, 18 Dec 2017 18:09:20 +0200	[thread overview]
Message-ID: <1890794.8U6CWPsXmq@avalon> (raw)
In-Reply-To: <CADnq5_N1f40k6+LTJU=ckxdpOPj1oGB6y1CkYDCOck42J2D+nA@mail.gmail.com>

Hi Alex,

On Friday, 15 December 2017 03:57:48 EET Alex Deucher wrote:
> On Thu, Dec 14, 2017 at 3:30 PM, Daniel Vetter wrote:
> > DK put some nice docs into the commit introducing driver private
> > state, but in the git history alone it'll be lost.
> > 
> > Also, since Ville remove the void* usage it's a good opportunity to
> > give the driver private stuff some tlc on the doc front.
> > 
> > Finally try to explain why the "let's just subclass drm_atomic_state"
> > approach wasn't the greatest, and annotate all those functions as
> > deprecated in favour of more standardized driver private states. Also
> > note where we could/should extend driver private states going forward
> > (atm neither locking nor synchronization is handled in core/helpers,
> > which isn't really all that great).
> > 
> > Cc: Harry Wentland <harry.wentland@amd.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: Rob Clark <robdclark@gmail.com>
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: Ben Skeggs <bskeggs@redhat.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > 
> >  Documentation/gpu/drm-kms.rst |  6 ++++++
> >  drivers/gpu/drm/drm_atomic.c  | 45 +++++++++++++++++++++++++++++++++++---
> >  include/drm/drm_atomic.h      | 28 +++++++++++++++++++++++++++
> >  include/drm/drm_mode_config.h |  9 +++++++++
> >  4 files changed, 85 insertions(+), 3 deletions(-)

[snip]

> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 37445d50816a..15e1a35c74a8 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c

[snip]

> > +/**
> > + * DOC: handling driver private state
> > + *
> > + * Very often the DRM objects exposed to userspace in the atomic modeset
> > api + * (&drm_connector, &drm_crtc and &drm_plane) do not map neatly to
> > the + * underlying hardware. Especially for any kind of shared resources
> > (e.g. shared + * clocks, scaler units, bandwidth and fifo limits shared
> > among a group of + * planes or CRTCs, and so on) it makes sense to model
> > these as independent + * objects. Drivers then need to similar state
> > tracking and commit ordering for + * such private (since not exposed to
> > userpace) objects as the atomic core and + * helpers already provide for
> > connectors, planes and CRTCs.
> 
> This last sentence doesn't quite parse.  I think it should be as follows:
> 
> Drivers then need to do similar state tracking and commit ordering for
> such private (since not exposed to userpace) objects as the atomic core that
> helpers already provide for DRM objects (connectors, planes and CRTCs).

I think Daniel meant

"Drivers then need similar state tracking and commit ordering for such private 
(since not exposed to userpace) objects as the atomic core and helpers already 
provide for connectors, planes and CRTCs."

[snip]

-- 
Regards,

Laurent Pinchart

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