Re: [PATCH 11/11] [v4] drm/i915/bdw: Ensure a context is loaded before RC6

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Thu, 20 Mar 2014 10:30:32 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> On Thu, 20 Mar 2014 14:42:32 +0100
> Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Wed, Mar 19, 2014 at 05:41:37PM -0700, Ben Widawsky wrote:
> > > I can't say it's completely unexpected that this would be your response,
> > > but I do feel like you've ignored my argument that this is better than
> > > the current situation. Not merging this patch only keeps things bad.
> > > 
> > > So I'd like you to re-consider merging this patch instead of waiting for
> > > the perfect solution. This patch requires a lot less review than the
> > > real fix. It has been tested by several people (I can ask them to put
> > > their reviewed by on it). It enables rc6 for people today, and this
> > > includes pc7, and deeper package and C states. It's very easy to revert
> > > if/when we get a real fix. Real users benefit from this patch. Real
> > > users are not hurt by this patch because if userspace is submitting bad
> > > state setup, they'll have the same or worse experience than failing RC6.
> > > 
> > > As an aside, this needs to come before I enable rc6 anyway. So the order
> > > way bad.
> > 
> > I fully agree with your assessment on technical reasons. The problem is
> > that I've just been forced through an exercise of "merge this now because
> > it works, people have tested it and we really, really, really need it to
> > move forward" and it didn't go down well.
> > 
> > Which means for the foreseeable future I'll reject patches when it looks
> > like a few too many rolls of ducttape have been involved in their
> > construction. I'd prefer if we could move more towards a merge early or at
> > least integration-test early model, but currently that's not a viable
> > model for me.
> > 
> > Note that this is not an iron rule at all, e.g. with psr I've just told
> > Rodrigo that I want the current state of affairs finalized for merging
> > instead of trying to hunt for the perfect patches. The reason for that was
> > that I think the remaining issues in psr support are well-understood and
> > we have solid test-coverage to make sure we don't fumble things. Also one
> > issue with the current psr patches is that they're way too conservative in
> > a few cases (i.e. wasting power), but for something tricky leaning towards
> > correctness is actually a feature. And bad power numbers tend to grab our
> > project managements attention. Overall the risks are a fairly clear
> > quantity.
> > 
> > In this area of rc6 and contexts though we have track record of not really
> > understanding issues. Which means that the risks here are unknown and
> > might be fairly big.
> 
> Do you think this patch falls into that class of issues?  It seems like
> it's a general improvement, even if it doesn't address the more
> general behavior we'd like (sooner than later really).
> 
> But blocking it until we have the full primitive emission seems like
> it's going to keep power consumption in a terrible state on BDW for the
> forseeable future... moreover I guess this is something we need going
> back forever for RC6 stability, at least according to the hw team.  So
> rather than blocking this, maybe we should commit it for all platforms!

Summarizing IRC discussion a bit: speaking generally of when some
future work blocks an existing fix or feature, we really need to make
sure someone is working on the broader task and make sure we track it
so it doesn't get lost, otherwise everyone loses.  The user loses
because a fix or feature isn't available, the author loses because
something gets blocked indefinitely, and upstream loses because either
a fix doesn't land or it does and the larger feature never gets
implemented because the pressure is off.

So with that, who wants to volunteer to update the 3D state emission
patch to include BDW and push it upstream?  Daniel promised to file a
JIRA task for this so our PM can track it, so someone will be
volunteered in the next week or so if we don't get it done before then.

-- 
Jesse Barnes, Intel Open Source Technology Center