On Thu, Jul 09, 2015 at 07:29:05PM +0100, Dave Gordon wrote:
> From: Alex Dai <yu....@intel.com>
> 
> This fetches the required firmware image from the filesystem,
> then loads it into the GuC's memory via a dedicated DMA engine.
> 
> This patch is derived from GuC loading work originally done by
> Vinit Azad and Ben Widawsky.
> 
> v2:
>     Various improvements per review comments by Chris Wilson
> 
> v3:
>     Removed 'wait' parameter to intel_guc_ucode_load() as firmware
>         prefetch is no longer supported in the common firmware loader,
>       per Daniel Vetter's request.
>     Firmware checker callback fn now returns errno rather than bool.
> 
> v4:
>     Squash uC-independent code into GuC-specifc loader [Daniel Vetter]
>     Don't keep the driver working (by falling back to execlist mode)
>         if GuC firmware loading fails [Daniel Vetter]
> 
> Issue: VIZ-4884
> Signed-off-by: Alex Dai <yu....@intel.com>
> Signed-off-by: Dave Gordon <david.s.gor...@intel.com>

I think this is it, one comment below.

> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index dbbb649..e020309 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5074,6 +5074,19 @@ i915_gem_init_hw(struct drm_device *dev)
>                       goto out;
>       }
>  
> +     /* We can't enable contexts until all firmware is loaded */
> +     ret = intel_guc_ucode_load(dev);

To stay in line with the current flow I think the request_firmware +
create fw bo object code should be move into gem_init so that gem_init_hw
is only responsible for loading the fw in the bo into hw.

That will take care of not trying to re-request the firmware from
userspace in resume/gpu reset code, which should simplify the status
handling a lot.

Also with just declaring the gpu wedged we could instead move the check
below into the init_hw part of the guc load process. That would nicely
encapsulate that decision and I'd expect take care of the other status
codes - in the end callers really only care about -EIO or not here.

But imo we can polish that after merging. All my other higher-level
concerns with this have been addressed, so I think this is good to go in
after detailed code review.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to