On Wed, Jan 13, 2016 at 02:11:42PM +0000, Tvrtko Ursulin wrote: > > On 13/01/16 13:36, Imre Deak wrote: > >On ke, 2016-01-13 at 12:46 +0000, Tvrtko Ursulin wrote: > >>On 11/01/16 16:56, Chris Wilson wrote: > >>>On Mon, Jan 11, 2016 at 05:36:46PM +0200, Ville Syrjälä wrote: > >>>>On Mon, Jan 11, 2016 at 03:16:16PM +0000, Tvrtko Ursulin wrote: > >>>>>Don't know, I leave this one to whoever grabbed the lock around > >>>>>intel_init_gt_powersave in the first place. Maybe there was a > >>>>>special > >>>>>reason.. after git blame od intel_display.c eventually > >>>>>completed, adding > >>>>>Imre and Ville to cc. > >>>> > >>>>Hmm. I don't recall the details anymore, but looking at the code > >>>>pushing > >>>>the locking down to valleyview_setup_pctx() looks entirely > >>>>reasonable to > >>>>me. > >>> > >>>iirc, this locking only exists to keep the WARN() at bay. But it is > >>>pedagogical, I guess. > >> > >>Don't really know this area, but what about the > >>intel_gen6_powersave_work->valleyview_enable_rps- > >>>valleyview_check_pctx > >>which dereferences the dev_priv->vlv_pctx, which is set/cleared in > >>valleyview_setup_pctx/valleyview_cleanup_pctx, which would now be > >>outside both struct_mutex and the rps lock? > > > >dev_priv->vlv_pctx is not protected on the premise that the driver > >init/cleanup functions can't race and gen6_powersave_work() is > >scheduled only after intel_init_gt_powersave() and flushed > >before intel_cleanup_gt_powersave(). > > > >rps_lock protects the RPS HW accesses themselves and struct_mutex was > >taken for the GEM allocation. Taking it at high level around > >intel_init_gt_powersave() was kind of a copy&paste in the commit you > >found, there is more on that in 79f5b2c75992. > > Thanks for digging this out. > > It is more involved than what Chris pasted then, and while I hoped > to be able to quickly shove that into a patch, I cannot allow the > time for more the extensive analysis. Imre just confirmed that struct_mutex is only for the GEM allocations in the vlv_setup_ctx, right Imre? -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
