[Intel-gfx] [PATCH v3 7/8] drm: Connector helper function to release resources

[Daniel Vetter]

On Fri, Feb 24, 2017 at 12:52:53AM +0000, Pandiyan, Dhinakaran wrote:
> On Thu, 2017-02-16 at 09:09 +0000, Lankhorst, Maarten wrote:
> > Daniel Vetter schreef op di 14-02-2017 om 20:51 [+0100]:
> > > On Mon, Feb 13, 2017 at 10:26 PM, Pandiyan, Dhinakaran
> > > <dhinakaran.pandiyan at intel.com> wrote:
> > > > On Mon, 2017-02-13 at 09:05 +0000, Lankhorst, Maarten wrote:
> > > > > Pandiyan, Dhinakaran schreef op do 09-02-2017 om 18:55 [+0000]:
> > > > > > > Could we deal with the VCPI state separately in
> > > > > > > intel_modeset_checks,
> > > > > > > like we do with dpll?
> > > > > > 
> > > > > > We'd want to release the VCPI slots before they are acquired in
> > > > > > ->compute_config(). intel_modeset_checks() will be too late to
> > > > > > release
> > > > > > them. Are you suggesting both acquiring and releasing slots
> > > > > > should be
> > > > > > done in intel_modeset_checks()?
> > > > > 
> > > > > That makes things a bit more nasty. Maybe add a
> > > > > conn_funcs->atomic_check that always gets called, something like
> > > > > I did
> > > > > below?
> > > > > 
> > > > > I'd love to use it for some atomic connector properties too.
> > > > 
> > > > 
> > > > Adding and unconditionally calling conn_funcs->atomic_check()
> > > > should be
> > > > doable. It also follows the pattern we have for encoders and CRTCs.
> > > > But
> > > > I'll have to move the connector->state->crtc state checks inside
> > > > the
> > > > function.
> > > 
> > > Adding ->atomic_check that's unconditionally called sounds troubling,
> > > because all the other ->atomic_check functions are _only_ called when
> > > enabling stuff. ->atomic_release sounds much better to me, and from a
> > > helper pov DK's patch above is the right place.
> > 
> > Having an atomic check would be nice for implementing connector
> > properties. Some of them may need to be validated regardless of crtc.
> > 
> 
> Can we add this later when we need state validation that is appropriate
> for an ->atomic_check()? 

+1 on not solving problems we don't have yet :-) If we'd write code for
every eventuality that we can come up with, we'd get nothing done. And
ime, such unused code will also be full of bugs.

Discussing issues like this is still good and useful, just to make sure we
have a coherent plan for the eventual future, once it happens.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch