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

Jesse Barnes
2014-03-20T17:29:46Z

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!

--
Jesse Barnes, Intel Open Source Technology Center