Re: [PATCH v3] drm/i915: resize the GuC WOPCM for rc6

From: Nick Hoath <nicholas.hoath@intel.com>
To: "Gordon, David S" <david.s.gordon@intel.com>,
	"Antoine, Peter" <peter.antoine@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Cc: "Vivi, Rodrigo" <rodrigo.vivi@intel.com>
Subject: Re: [PATCH v3] drm/i915: resize the GuC WOPCM for rc6
Date: Mon, 16 May 2016 15:12:54 +0100	[thread overview]
Message-ID: <5739D566.7030509@intel.com> (raw)
In-Reply-To: <572C8BB1.7000807@intel.com>

On 06/05/2016 13:18, Gordon, David S wrote:
> On 06/05/16 10:37, Nick Hoath wrote:
>> On 05/05/2016 16:04, Dave Gordon wrote:
>>> On 05/05/2016 15:02, Antoine, Peter wrote:
>>>> The attached version still does not explain that the WOPCM_TOP is to
>>>> tell the GuC not to use that space.
>>>
>>> That's NOT what WOPCM_TOP means. The GuC is allowed to use the space up
>>> to the value stored in the GUC_WOPCM_SIZE register (as the comment above
>>> the #define says). Architecturally, this is allowed to be any value
>>> greater than (16K+sizeof internal SRAM (64, 128, or 256K)) and less than
>>> or equal to GUC_WOPCM_TOP (which is a platform-independent constant), so
>>> we normally choose the maximm allowed. Howver on BXT, we need to leave
>>> some space at the top for the RC6 image, hence the logic (and comments!)
>>> in guc_wopcm_size().
>>>
>>>> The extra information does not aid anybody as the information is used
>>>> internally within the GuC.
>>> It may help the next person who has to figure out what's gone wrong on
>>> some future chip that needs more than 64K for RC6!
>>>
>>> .Dave.
>>>>
>>>> But, I have not actual objection to the patch.
>>>>
>>>> Peter.
>>>>
>>>
>> Unfortunately Dave's patch locked my test system on bootup, so I've t-b
>> & r-b'd Peter's.
>
> They're equivalent, unless your firmware happens to be between 458752
> and 491520 bytes in size (in which case you have a problem anyway).
>
> To check, I've run both versions, with debug printing the value chosen
> (on SKL) and the value that would have been chosen on BXT, and they're
> identical (and both work). So I think your build had some other problem
> unrelated to the specific patch.
>
> I've no problem with using Peter's patch for now, but it's not just a
> matter of the comments; there's also the other use(s) of
> GUC_WOP_(TOP,SIZE_VALUE), with ad-hoc additions or subtractions. So it
> still needs fixing properly.
>
> .Dave.
>
After a rebuild & a retest, Dave's patch works fine. Therefore for 
"drm/i915/bxt: reserve space for RC6 in the the GuC WOPCM":
Tested-by: Nick Hoath <nicholas.hoath@intel.com>
Reviewed-by: Nick Hoath <nicholas.hoath@intel.com>


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx