[Intel-gfx] [PATCH 08/26] drm: Consolidate connector arrays in drm_atomic_state

[Ville Syrjälä]

On Mon, May 30, 2016 at 05:33:56PM +0200, Daniel Vetter wrote:
> On Mon, May 30, 2016 at 06:25:50PM +0300, Ville Syrjälä wrote:
> > On Mon, May 30, 2016 at 05:13:56PM +0200, Daniel Vetter wrote:
> > > On Mon, May 30, 2016 at 05:59:24PM +0300, Ville Syrjälä wrote:
> > > > On Sun, May 29, 2016 at 08:35:05PM +0200, Daniel Vetter wrote:
> > > > > It's kinda pointless to have 2 separate mallocs for these. And when we
> > > > > add more per-connector state in the future it's even more pointless.
> > > > > 
> > > > > Right now there's no such thing planned, but both Gustavo's per-crtc
> > > > > fence patches, and some nonblocking commit helpers I'm playing around
> > > > > with will add more per-crtc stuff. It makes sense to also consolidate
> > > > > connectors, just for consistency.
> > > > > 
> > > > > In the future we can use this to store a pointer to the preceeding
> > > > > state, making an atomic update entirely free-standing. This will be
> > > > > needed to be able to queue them up with a depth > 1.
> > > > > 
> > > > > Cc: Gustavo Padovan <gustavo at padovan.org>
> > > > > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/drm_atomic.c        | 27 +++++++++------------------
> > > > >  drivers/gpu/drm/drm_atomic_helper.c |  2 +-
> > > > >  include/drm/drm_atomic.h            | 10 +++++-----
> > > > >  include/drm/drm_crtc.h              | 11 +++++++----
> > > > >  4 files changed, 22 insertions(+), 28 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > > > > index 3ff1ed7b33db..a6395e9654af 100644
> > > > > --- a/drivers/gpu/drm/drm_atomic.c
> > > > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > > > @@ -44,7 +44,6 @@
> > > > >  void drm_atomic_state_default_release(struct drm_atomic_state *state)
> > > > >  {
> > > > >  	kfree(state->connectors);
> > > > > -	kfree(state->connector_states);
> > > > >  	kfree(state->crtcs);
> > > > >  	kfree(state->crtc_states);
> > > > >  	kfree(state->planes);
> > > > > @@ -139,15 +138,15 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state)
> > > > >  	DRM_DEBUG_ATOMIC("Clearing atomic state %p\n", state);
> > > > >  
> > > > >  	for (i = 0; i < state->num_connector; i++) {
> > > > > -		struct drm_connector *connector = state->connectors[i];
> > > > > +		struct drm_connector *connector = state->connectors[i].ptr;
> > > > 
> > > > 'ptr' is not a very good name.
> > > 
> > > Suggestions for something better? I was lacking good inspiration ...
> > 
> > Maybe just 'connector' ?
> 
> state->connectors[i].connector is really long, and makes a lot of code
> look ugly. "obj" might be a bit better than "ptr" at least. Something
> else?

How often are you expecting to have to type this anyway? Using any
kind of generic name here will make life harder for cscope users.
Atomic is really bad in this regard, escecially with all the identically
named function pointers. It's seriosuly hard to navigate that maze
these days. Someone should do a bit of renaming of stuff to make
things more unique.

-- 
Ville Syrjälä
Intel OTC