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 ... .Dave. _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx