Re: [PATCH v11 5/6] drm/i915/guc: Check the locking status of GuC WOPCM registers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 03/01/2018 05:37 AM, Michal Wajdeczko wrote:
On Thu, 01 Mar 2018 02:01:39 +0100, Jackie Li <yaodong.li@xxxxxxxxx> wrote:


+
+    err = write_and_verify(dev_priv, GUC_WOPCM_SIZE,
+                   dev_priv->wopcm.guc.size,

you should use wopcm-> instead dev_priv->wopcm. (same below)

+                   GUC_WOPCM_SIZE_MASK | GUC_WOPCM_SIZE_LOCKED,
+                   GUC_WOPCM_SIZE_LOCKED);

bikeshed: we should set BASE first, then SIZE ;)
I'd like to keep the sequence it as how the existing code is doing this.
Plus It is only called once during init, and now it works perfectly.:-)

+    if (err)
+        goto err_out;
+
+    huc_agent = USES_HUC(dev_priv) ? HUC_LOADING_AGENT_GUC : 0;
+    mask = GUC_WOPCM_OFFSET_MASK | GUC_WOPCM_OFFSET_VALID | huc_agent;
+    err = write_and_verify(dev_priv, DMA_GUC_WOPCM_OFFSET,
+                   dev_priv->wopcm.guc.base | huc_agent, mask,
+                   GUC_WOPCM_OFFSET_VALID);

as the only supported HuC scenario for us is always with
HUC_LOADING_AGENT_GUC, maybe we should always set this bit,
but only add to mask for check conditionally? otherwise we
couldn't run first without and later with HuC... just asking

I assume what you are asking is how to deal with the use case that
we first run without HuC firmware, then we unload the driver module
and come back with a HuC FW?

In this case, we need to also recalculate the GuC region and we need
also the reg values accordingly. Unfortunately, we cannot do it now once
the regs were locked.

with function description and use wopcm pointer fixed,

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Thanks again for the review.:-)
-Jackie

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux