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

[Peter Antoine <peter.antoine@intel.com>]

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.
> 
> 
> 
>

--
    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