On 04/28/2015 08:12 AM, Dave Gordon wrote:
On 23/04/15 18:48, Dave Gordon wrote:
> On 17/04/15 22:21, yu.dai@xxxxxxxxx wrote:
>> From: Alex Dai <yu.dai@xxxxxxxxx>
>>
>> Add GuC firmware loader. It uses the unified firmware loader to
>> fetch firmware blob first, then load to hw in driver main thread.
>>
>> Issue: VIZ-4884
>> Signed-off-by: Alex Dai <yu.dai@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/Makefile | 3 +-
>> drivers/gpu/drm/i915/i915_dma.c | 6 +
>> drivers/gpu/drm/i915/i915_drv.h | 7 +
>> drivers/gpu/drm/i915/i915_gem_stolen.c | 10 +
>> drivers/gpu/drm/i915/i915_reg.h | 4 +-
>> drivers/gpu/drm/i915/intel_guc.h | 103 ++++++++++
>> drivers/gpu/drm/i915/intel_guc_loader.c | 348 ++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/intel_uc_loader.c | 3 +
>> drivers/gpu/drm/i915/intel_uc_loader.h | 4 +
>> 9 files changed, 486 insertions(+), 2 deletions(-)
>> create mode 100644 drivers/gpu/drm/i915/intel_guc.h
>> create mode 100644 drivers/gpu/drm/i915/intel_guc_loader.c
>
> Hi Alex,
>
> some comments interspersed below ...
>
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>> index e44116f..5d50b5b 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -465,6 +465,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
>>
>> cleanup_gem:
>> mutex_lock(&dev->struct_mutex);
>> + intel_guc_ucode_fini(dev);
>> i915_gem_cleanup_ringbuffer(dev);
>> i915_gem_context_fini(dev);
>> mutex_unlock(&dev->struct_mutex);
>> @@ -861,6 +862,10 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>>
>> intel_uncore_init(dev);
>>
>> + intel_wopcm_init(dev);
>> +
>> + intel_guc_ucode_init(dev);
>
> Can we put this any earlier? We might as well get the async fetch
> started as early as possible ...
Also, does intel_wopcm_init() meed to be called only once, during driver
load? Or should it be called after a reset as well? Maybe it should be
called from intel_guc_load_ucode() ? In which case it could live in one
of the GuC-specific files and be static :)
Possibly it belongs near the gem-guc-object allocator, since the setting
of this register and the selection of GuC-accessible offsets within the
GTT are related ...
In my local tree, this has been moved to right before load firmware.
This wopcm_init() was introduced for huc firmware loading. We must load
huc before guc. Since we don't have plan to upsteam huc frimware yet, as
you suggested, we can move this wopcm_init() to guc_load_ucode() make
code clean. In case we upstream huc firmware in future, we can put it
before huc loading.
Alex
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
