Quoting Fernando Pacheco (2019-04-09 23:53:08) > > On 4/9/19 2:53 PM, Chris Wilson wrote: > > Quoting Fernando Pacheco (2019-04-09 22:31:01) > >> +int intel_uc_fw_ggtt_pin(struct intel_uc_fw *uc_fw, > >> + struct i915_ggtt *ggtt, u64 start) > >> +{ > >> + struct drm_i915_gem_object *obj = uc_fw->obj; > >> + int err; > >> + > >> + err = i915_gem_object_set_to_gtt_domain(obj, false); > > Currently requires struct_mutex, and is not required as we can ensure > > the pages are flushed on creation. > > My intent was to maintain what was being done before > but just doing it earlier. > > But if it's not required.. More so that it's illegal in the global reset context :) There are patches on the list that remove the struct_mutex for this operation (eek, no, ignore that you aren't allowed to take that lock inside reset either!), but with a bit of care we shouldn't need the set-to-gtt-domain at all as we can do the flush trivially (if it's even required). > >> + if (err) { > >> + DRM_DEBUG_DRIVER("%s fw set-domain err=%d\n", > >> + intel_uc_fw_type_repr(uc_fw->type), err); > >> + return err; > >> + } > >> + > >> + err = i915_gem_object_pin_pages(obj); > > I'm pretty sure we don't need to pin the pages here, as the caller > > should be holding the pages already for the duration of the bind. > > > > So I think this should just reduce to the ggtt bind. > > I might be misunderstanding, so could you please clarify > what you mean by "should be holding"? Are you stating > that the caller already holds the pages? To copy the firmware image into the pages, those pages must exist. To prevent those pages disappearing, we must have kept them around (i.e pinned). So we know by construction of the uc_fw object we always have the pages, and could skip bumping the pin-count around the xfer. Therefore we only need to bind the existing firmware pages into the GGTT to perform the dma xfer (after satisfying ourselves that the pages are indeed flushed). -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx