Re: [PATCH] drm/i915/chv: WaDisablePwrmtrEvent:chv on CHV only

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Jun 12, 2014 at 01:33:32PM +0300, Jani Nikula wrote:
> On Tue, 10 Jun 2014, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > On Tue, 10 Jun 2014, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >> On Tue, Jun 10, 2014 at 06:34:18PM +0300, Jani Nikula wrote:
> >>> On Tue, 10 Jun 2014, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> >>> > On Mon, 09 Jun 2014, Damien Lespiau <damien.lespiau@intel.com> wrote:
> >>> >> On Mon, Jun 09, 2014 at 10:06:49AM -0700, Tom.O'Rourke@intel.com wrote:
> >>> >>> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
> >>> >>> 
> >>> >>> In gen8_enable_rps, don't write CHV registers unless IS_CHERRYVIEW.
> >>> >>> 
> >>> >>> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
> >>> >>
> >>> >> A lovely catch.
> >>> >
> >>> > Sadly gen8_enable_rps does not get called on chv, so the fix is wrong.
> >>> 
> >>> To elaborate, I think we need a patch dropping the wa altogether (which
> >>> we can queue for 3.15 through stable because the change affects
> >>> broadwell) and another patch, if needed, adding the wa in the chv
> >>> specific function.
> >>
> >> This is just a merge mishap in one the chv patches. Someone just
> >> needs to send a patch that moves the misapplied stuff to the
> >> appropriate chv function.
> >
> > Right. So my first comment was correct, and my elaboration total
> > bullcrap. This is not present in 3.15, but we've queued the screwup for
> > 3.16. Thanks for the correction Ville.
> 
> Argh. I'm really confusing myself and others here. Please bear with me.
> 
> So we've added
> 
> commit e4443e459ccf43f2c139358400365fd6a839d40d
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date:   Wed Apr 9 13:28:41 2014 +0300
> 
>     drm/i915/chv: Add a bunch of pre production workarounds
> 
> which contains the chv specific w/a in bdw code. This is now going to
> 3.16, and we need to fix this for 3.16 through
> drm-intel-fixes. Effectively the hunk touching gen8_enable_rps() from
> Tom's new patch [1]. Right?
> 
> However the new patch from Tom moves those bits to
> cherryview_enable_rps(), which is only present since
> 
> commit 38807746fa2ce44b79957ff07813d10fcaf3d311
> Author: Deepak S <deepak.s@linux.intel.com>
> Date:   Fri May 23 21:00:15 2014 +0530
> 
>     drm/i915/chv: Enable Render Standby (RC6) for Cherryview
> 
> and queued for 3.17. So we need another patch adding the bits to
> cherryview_enable_rps() on top of drm-intel-next-queued, effectively the
> second hunk from Tom's new patch. Right?
> 
> So Tom, please split your patch in two, one on top of drm-intel-fixes,
> and another on top of drm-intel-next-queued, and I think we'll be fine.

I've already merged Tom's patch to dinq since it's fully needed there. So
I think we just need a new patch version for -fixes only and resolve the
mess in a merge (shouldn't cause one). If we split the patch also for dinq
then I need to rebase/merge again. I guess you could simply apply Tom's
fix to -fixes and drop the unecessary hunk yourself.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch