[Intel-gfx] [PATCH v4 3/5] drm/i915/guc: Implement dynamic WOPCM partitioning

[Yaodong Li]

On 12/13/2017 01:34 PM, Michal Wajdeczko wrote:
> On Wed, 13 Dec 2017 19:19:06 +0100, Yaodong Li <yaodong.li at intel.com> 
> wrote:
>
>> On 12/13/2017 01:11 AM, Joonas Lahtinen wrote:
>>> On Tue, 2017-12-12 at 14:56 -0800, Jackie Li wrote:
>>>> Hardware may have specific restrictions on GuC WOPCM partition
>>>> size versus HuC firmware size. With static WOPCM partitioning,
>>>> there's no way to adjust the GuC WOPCM partition size based on
>>>> the actual HuC firmware size, so that GuC/HuC loading failure
>>>> would occur even if there was enough WOPCM space for both
>>>> GuC and HuC firmware.
>>> WOPCM being a shared feature of the hardware, it should not go under
>>> intel_guc_ prefix.
>>>
>>> There should be a clear division of what is specific to GuC feature
>>> only and what is just a feature that happens to be used by GuC (and
>>> equally can be used by HuC too).
>> the intel_guc_wopcm here only refers to the wopcm used by
>> GuC, this structure only defines the GuC related wopcm info.
>> (wopcm partition for GuC). We only need to set these values
>> (defined in this structure) to GuC registers. And this structure
>> should never be touched if GuC was disabled. so it should be
>> a part of GuC.
>>
>
> But note that yours intel_guc_wopcm is just one of many wopcm partitions.
> I think it would be a good idea to create "intel_wopcm.c|h" and keep
> all related code and data there (including verification of early setup
> done by bios, wopcpm reporting, partitioning).
>
> Then we can do rest of the programming right there or just take values 
> that
> will be programmed individually by interested components (but former is
> preferred to avoid spreading single feature code over too many places)
>
The KMD only needs to take care of the setup of the GuC WOPCM partition. 
Other
HW WOPCM (e.g HuC) usages are all transparent to kernel driver. Plus, 
the GuC WOPM
partitioning is needed only when GuC is enabled and uc firmwares are 
loaded correctly.
The only reason for us to have an intel_wopcm is to maintain the overall 
WOPCM info
such as WOPCM size and base. However, it's not necessary since we can 
reuse existing
driver code to get these info.

Regards,
-Jackie
> Michal