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

From: Nick Hoath <nicholas.hoath@intel.com>
To: Peter Antoine <peter.antoine@intel.com>,
	Dave Gordon <david.s.gordon@intel.com>
Cc: "intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Vivi, Rodrigo" <rodrigo.vivi@intel.com>
Subject: Re: [PATCH v3] drm/i915: resize the GuC WOPCM for rc6
Date: Fri, 6 May 2016 10:36:26 +0100	[thread overview]
Message-ID: <572C659A.5070800@intel.com> (raw)
In-Reply-To: <alpine.DEB.2.10.1605060734170.3052@peterant-linux2>

On 06/05/2016 08:01, Peter Antoine wrote:
> On Thu, 5 May 2016, 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().
> Yes, the firmware can use upto GUC_WOPCM_TOP and to leave the rest alone.
>>
>> 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!
>
> You hid a if statement in a function (making the code harder to read and
> more prone to error). Where maybe a slightly clearer comment was required.
>
> And this patch has been held up two weeks just for a better comment.
>
> Peter.
>>
>> .Dave.
>
> And what if the next reserved space is not for RC6?
>
>>
>>
>> But, I have not actual objection to the patch.
>>
>> Peter.
>>
>>
>>
>>
Tested-by: Nick Hoath <nicholas.hoath@intel.com>
Reviewed-by: Nick Hoath <nicholas.hoath@intel.com>
>
> --
>     Peter Antoine (Android Graphics Driver Software Engineer)
>     ---------------------------------------------------------------------
>     Intel Corporation (UK) Limited
>     Registered No. 1134945 (England)
>     Registered Office: Pipers Way, Swindon SN3 1RJ
>     VAT No: 860 2173 47
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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