Hi Daniel, On Tue, Oct 06, 2015 at 01:10:00PM +0200, Daniel Vetter wrote: > On Tue, Oct 06, 2015 at 12:10:48PM +0200, Lukas Wunner wrote: > > On Tue, Oct 06, 2015 at 09:27:24AM +0200, Daniel Vetter wrote: > > > Also while reading the patch I realized that the new lock really protects > > > hw state (instead of our software-side tracking and driver resume/suspend > > > code). And at least myself I have no idea what vgasr means, so what about > > > renaming it to hw_mutex? We have this pattern of low-level locks to avoid > > > concurrent hw access in a lot of places like i2c, dp_aux, and it's very > > > often called hw_lock or similar. > > > > Hm, hw_lock sounds a bit ambiguous. > > > > vgasr_mutex is really a catch-all that protects access to the vgasr_priv > > structure and also protects various hardware state. (Power state of each > > GPU, mux state.) Up until now this approach worked fine, it looks like > > there was no need for additional locks. We may need to move to more > > fine-grained locking as new features get added to vga_switcheroo. > > The newly introduced ddc_lock is a first step and I think is aptly > > named since it only protects the mux state for the DDC lines. > > Oops sorry mixed up the names. I meant rename the ddc_lock to hw_lock > since it protects a bit more than just the ddc (it protects the entire hw > switch state since we grab it both for dcc switching and for full-blown > switching of everything). Interesting observation. Actually the intention is to use ddc_lock to only lock hardware state of the DDC switch, but you're right, it also locks hardware state of the panel switch. That's an unintended consequence of the ->switchto callback in apple-gmux also switching DDC, and the need to avoid races because of this. Now that you mention it I'm contemplating removing DDC switching from the ->switchto callback in apple-gmux. Then I don't need to take the ddc_lock when switching the full panel. > > > Also, the revised docbook patch seems to be missing from this iteration, > > > please follow up with that one. > > > > The docbook patch posted September 17 automatically picks up the > > kernel-doc for the new functions through the !E directive. > > I asked for the inclusion to be moved to drm.tmpl instead of creating a > new docbook. Your argument was that including it in drm.tmpl will increase the chances it's read. Quite honestly I think that reasoning is a bit thin. One might as well argue that people won't find the information in the juggernaut of 180 kByte XML text that is drm.tmpl. The (honest) question is this, vga_switcheroo is not located in the DRM tree, so it's a separate subsystem. Then why should someone be looking for its documentation in the DRM DocBook? > > By the way, Jani has kindly provided a Reviewed-By: for patch 6 of > > the vga_switcheroo docs series. The patch is not dependent on the > > preceding patch 5 which is awaiting an ack from Takashi, so could > > be merged now: > > [PATCH 06/15] vga_switcheroo: Use enum vga_switcheroo_state instead > > > > Patches 7 and 8 are similarly trivial cleanups: > > [PATCH 07/15] vga_switcheroo: Use VGA_SWITCHEROO_UNKNOWN_ID instead > > [PATCH 08/15] vga_switcheroo: Use enum vga_switcheroo_client_id > > > > And patch 10 has gotten a Reviewed-By: from Takashi: > > [PATCH 10/15] ALSA: hda - Spell vga_switcheroo consistently > > Hm those patches aren't in this series, that makes it really hard for me > to know what's still pending. I think it would be best to resend > _everything_, so including docs and cleanups and all that. Otherwise I > think this will take a lot longer than necessary to pull in. I updated my vga_switcheroo_docs branch on GitHub as the Reviewed-Bys came in and just rebased it to current topic/drm-misc, so you can pull from there if you're comfortable with that: https://github.com/l1k/linux/commits/vga_switcheroo_docs If on the other hand you prefer merging stuff via the mailing list, I'll be happy to resend. Thanks, Lukas _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
