Re: [PATCH v2 07/16] drm/i915: Add vblank based delayed watermark update mechanism

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Jun 04, 2014 at 06:10:44PM +0200, Daniel Vetter wrote:
> On Wed, Jun 04, 2014 at 11:01:01AM -0300, Paulo Zanoni wrote:
> > > This function is only called at init/resume. It populates the software
> > > state with something that matches the current hardware state. I guess
> > > a comment explaning the purpose of the function is the best we can do
> > > here, or do you have a better idea?
> > 
> > The problem is that we can use the get_hw_state functions not only to
> > check driver state a init/resume, but also to do state tracking
> > assertions at certain points of the code. Since most (all?) the other
> > HW state readout functions don't have side-effects, there's a
> > possibility that someone may add code to do HW state assertion at some
> > points, and just call these things without realizing the potential
> > side effects. A comment would help, but moving the assignment to
> > another place would also solve the problem for me. You choose.
> 
> We already do that in other places, e.g. the edp bit depth hacks we have.
> But a comment might be justified.
> 
> I haven't looked at the details here, just a high-level comment that we do
> play such ugly tricks already. And I agree that it's unexpected.

Well IIRC I didn't even call this function get_hw_state() until
"someone" asked me to rename it ;) I could rename it back to whatever
it was originally. It was never meant to be just a "read hardware state
into a temp structure" type of thing since it's been updating
intel_crtc->wm.active from the start.

With the rename I should be able to drop the ugly underscore from
_ilk_wm_get_hw_state().

-- 
Ville Syrjälä
Intel OTC