diff mbox

drm/i915/huc: Use helper function while waiting for DMA completion

Message ID 20171024105056.43276-1-michal.wajdeczko@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michal Wajdeczko Oct. 24, 2017, 10:50 a.m. UTC
Waiting for DMA status register can be done with dedicated function.
Lets use it as additional bonus will be smaller driver footprint.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_huc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Chris Wilson Oct. 24, 2017, 11:23 a.m. UTC | #1
Quoting Michal Wajdeczko (2017-10-24 11:50:56)
> Waiting for DMA status register can be done with dedicated function.
> Lets use it as additional bonus will be smaller driver footprint.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_huc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
> index c8a48cb..98d1725 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -151,7 +151,7 @@ static int huc_ucode_xfer(struct intel_uc_fw *huc_fw, struct i915_vma *vma)
>         I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(HUC_UKERNEL | START_DMA));
>  
>         /* Wait for DMA to finish */
> -       ret = wait_for((I915_READ(DMA_CTRL) & START_DMA) == 0, 100);
> +       ret = intel_wait_for_register_fw(dev_priv, DMA_CTRL, START_DMA, 0, 100);
>  
>         DRM_DEBUG_DRIVER("HuC DMA transfer wait over with ret %d\n", ret);

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Aside, what's the serialisation so that we only try to load one fw at a
time?
-Chris
Michal Wajdeczko Oct. 24, 2017, 2:25 p.m. UTC | #2
On Tue, 24 Oct 2017 13:23:45 +0200, Chris Wilson  
<chris@chris-wilson.co.uk> wrote:

> Quoting Michal Wajdeczko (2017-10-24 11:50:56)
>> Waiting for DMA status register can be done with dedicated function.
>> Lets use it as additional bonus will be smaller driver footprint.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_huc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_huc.c  
>> b/drivers/gpu/drm/i915/intel_huc.c
>> index c8a48cb..98d1725 100644
>> --- a/drivers/gpu/drm/i915/intel_huc.c
>> +++ b/drivers/gpu/drm/i915/intel_huc.c
>> @@ -151,7 +151,7 @@ static int huc_ucode_xfer(struct intel_uc_fw  
>> *huc_fw, struct i915_vma *vma)
>>         I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(HUC_UKERNEL |  
>> START_DMA));
>>
>>         /* Wait for DMA to finish */
>> -       ret = wait_for((I915_READ(DMA_CTRL) & START_DMA) == 0, 100);
>> +       ret = intel_wait_for_register_fw(dev_priv, DMA_CTRL, START_DMA,  
>> 0, 100);
>>
>>         DRM_DEBUG_DRIVER("HuC DMA transfer wait over with ret %d\n",  
>> ret);
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> Aside, what's the serialisation so that we only try to load one fw at a
> time?

It looks that we just wait right after starting DMA. And if we attempt to
next DMA while previous is still running then it will be likely silently
ignored by HW as control bit START_DMA would be still on.

If we want to be bullet-proof then maybe before we start programming new
DMA xfer we should verify that DMA hw is idle (and then wait little more
or just abort immediately)

Michal
Chris Wilson Oct. 26, 2017, 8:40 p.m. UTC | #3
Quoting Patchwork (2017-10-24 13:27:27)
> == Series Details ==
> 
> Series: drm/i915/huc: Use helper function while waiting for DMA completion
> URL   : https://patchwork.freedesktop.org/series/32528/
> State : failure
> 
> == Summary ==
> 
> Series 32528v1 drm/i915/huc: Use helper function while waiting for DMA completion
> https://patchwork.freedesktop.org/api/1.0/series/32528/revisions/1/mbox/
> 
> Test kms_pipe_crc_basic:
>         Subgroup read-crc-pipe-a:
>                 pass       -> INCOMPLETE (fi-glk-1)

Unrelated, but unusual GEM_BUG_ON, which I hope I've already fixed...

Pushed, thanks for the patch.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index c8a48cb..98d1725 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -151,7 +151,7 @@  static int huc_ucode_xfer(struct intel_uc_fw *huc_fw, struct i915_vma *vma)
 	I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(HUC_UKERNEL | START_DMA));
 
 	/* Wait for DMA to finish */
-	ret = wait_for((I915_READ(DMA_CTRL) & START_DMA) == 0, 100);
+	ret = intel_wait_for_register_fw(dev_priv, DMA_CTRL, START_DMA, 0, 100);
 
 	DRM_DEBUG_DRIVER("HuC DMA transfer wait over with ret %d\n", ret);