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

From: Dave Gordon <david.s.gordon@intel.com>
To: Nick Hoath <nicholas.hoath@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: Fri, 6 May 2016 13:18:57 +0100	[thread overview]
Message-ID: <572C8BB1.7000807@intel.com> (raw)
In-Reply-To: <572C65DD.1080500@intel.com>

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.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx