On Wed, 2017-12-13 at 14:59 -0800, Yaodong Li wrote: > On 12/13/2017 01:34 PM, Michal Wajdeczko wrote: > > On Wed, 13 Dec 2017 19:19:06 +0100, Yaodong Li <yaodong.li@xxxxxxxxx> > > 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. I'd go with Michal here, the WOPCM is its own entity in existence. Partitioning defintely sounds like it should be intel_wopcm stuff, which may yield intel_wopcm_partition under "guc", so then you are still able to reference "guc->wopcm.base" where it makes sense. And how that partition is programmed to GuC registers for it to be used, is then stuff to go under intel_guc. And then you have another intel_wopcm_partition for "huc". We should avoid incorrect abstractions, just to avoid a few lines of code. That's how the hardware features seem to exist, that's how we should map them in the code. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
