[Intel-gfx] [PATCH 1/2] drm/i915: Add code to accept valid locked WOPCM register values

[Yaodong Li]

On 03/26/2018 12:15 AM, Joonas Lahtinen wrote:
> Quoting Yaodong Li (2018-03-23 19:33:15)
>> As I said, I agree that we would likely solve the enable_guc=1 then
>> enable_guc=3 case with these changes which I think this the the only benefit
>> that we can get from the starting from the top way.
>> But my point is just like the from the bottom way, you are ignoring the
>> HuC FW size. e.g. you will need to grow the guc wopcm size for the hw
>> restrictions.
>> The problem is currently we do have this restriction on huc fw size v.s.
>> guc wopcm
>> size. In you solution, you are actually counting on the assumption that
>> guc fw size should be large enough so that we can ignore the huc fw size
>> and expect it still works when we set enable_huc=3. and my answer to
>> this is yes it will work for the cases that guc fw size is large enough, but
>> still risky to fail in case of guc fw size < huc_fw_size + 16K. and that
>> comes
>> to my point:
>> why not make life easier and accurate - If we decide to support dynamically
>> switching on/off huc fw loading and the fact we can get actual FW sizes,
>> why not drop all these assumptions and fix it in a way that we won't be
>> bothered by any FW side changes? :)
> Is not the GuC vs. HuC FW size limitation going to be fixed for upcoming
> platforms?
>
> My gut instinct would be to partition based only on the enabled FW sizes,
> whichever it be. Then just require a reboot if after that partitioning,
> changing the parameter causes the FW not to fit.
That's my thought too. I think it makes sense to reboot the system for
any FW updates, especially when we have these write-once registers.

I believe the main reason we wanted to support enable_guc=1 (GuC FW only)
then enable_guc=3 (GuC + HuC FW) without a system reboot is to facilitate
the debugging.

Regards,
-Jackie