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@xxxxxxxxx>
Reviewed-by: Nick Hoath <nicholas.hoath@xxxxxxxxx>
--
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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
