Lukas Wunner
2015-Aug-11 10:29 UTC
[Nouveau] [PATCH v2 00/22] Enable gpu switching on the MacBook Pro
This is a follow-up to the v1 posted in April:
http://lists.freedesktop.org/archives/dri-devel/2015-April/081515.html
Patches 1 - 17 enable GPU switching on the pre-retina MacBook Pro.
These were tested successfully by multiple people and solve two
tickets in Bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=88861
https://bugs.freedesktop.org/show_bug.cgi?id=61115
Patches 18 - 22 are a preview of how we're tackling retina support.
Those are marked experimental and are NOT ready to be merged yet.
Feedback on them is welcome.
The patches are based on drm-next.
They were tested on the following hardware (thanks a lot everyone!):
Tested-by: Paul Hordiienko <pvt.gord at gmail.com>
[MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina]
Tested-by: William Brown <william at blackhats.net.au>
[MBP 8,2 2011 intel SNB + amd turks pre-retina]
Tested-by: Lukas Wunner <lukas at wunner.de>
[MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina]
Tested-by: Bruno Bierbaumer <bruno at bierbaumer.net>
[MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress]
What's new:
* By default the MBP boots with the display switched to the discrete GPU
but it can be forced to the integrated GPU with an EFI boot variable.
Here's a handy tool for that: https://github.com/0xbb/gpu-switch
v1 didn't work in this configuration, v2 does.
* Reprobing if the inactive GPU initializes before the apple-gmux module:
v1 used Matthew Garrett's approach of adding a driver callback.
v2 simply generates a hotplug event instead. nouveau polls its outputs
every 10 seconds so we want it to poll immediately once apple-gmux
registers. That is achieved by the hotplug event. The i915 driver is
changed to behave identically to nouveau. (Right now it deletes LVDS
and eDP connectors from the mode configuration if they can't be probed,
deeming them to be ghosts.)
* Framebuffer recreation if the inactive GPU initializes before the
apple-gmux module (i.e. discarding the default 1024x768 fb and replacing
with a new one with the actual panel resolution): v1 only supported this
for i915, v2 has a generic solution which works with nouveau and radeon
as well. (Necessary if the discrete GPU is forced to be the inactive one
on boot via the EFI variable.)
* Generally lots of rough edges were smoothed to hopefully make the
patches more suitable for merging. E.g. there's a bug in i915 where
the SSC status set by BIOS is preserved too late and v1 only contained
a workaround, whereas v2 contains a proper fix in a separate commit.
The long journey towards retina support:
The pixel clock required for retina resolution is not supported by LVDS
(which was used on pre-retinas), necessitating eDP instead. Problem is,
the gmux register which switches the DDC lines on pre-retinas doesn't
switch the AUX channel on retinas. Disassembling the OS X driver revealed
that the gmux in retina MBPs gained an additional register 0x7f which gets
written to when setting up the eDP configuration. There was some hope that
this might switch the AUX channel. Alas, we tried writing various values
to that register but were unable to get the inactive GPU to talk to the
panel. The purpose of register 0x7f remains a mystery.
Teardowns of the first generation retina MBP name the NXP CBTL06142 and
TI HD3SS212 as multiplexers and according to the data sheets I've found,
neither supports switching the AUX channel separately from the main link.
Matthew Garrett had the idea of having the active GPU stash the EDID and
the first 8 bytes of the DPCD (receiver capabilities) and letting the
inactive GPU retrieve that data. I rebased and rewrote his patches and
got everything working, only to discover that the drivers are unhappy
with just 8 bytes of DPCD. They need full access to the DPCD to set up
their outputs. We could stash the entire DPCD but some parts of it are
mutable so the stashed data may become stale when the active GPU performs
writes to the DPCD.
So I had the idea of using the active GPU as a proxy to talk to the panel,
thus emulating switching of the AUX channel in software. We can leverage
the struct drm_dp_aux and i2c_adapter (for DDC) to achieve this, swapping
the inactive GPU's structs with those of the active GPU on the fly.
That approach is implemented in patches 18 - 22 but there are still some
driver issues that I'm debugging. The current status as per the latest
logs Bruno sent me is that i915 rejects the mode retrieved via proxying
with CLOCK_HIGH and nouveau aborts link training halfway through.
Bottom line is that it's not yet working but we're getting closer.
As a side effect, the pre-retinas gain a second way to initialize their
outputs: They can either use gmux to switch the DDC lines, or use the
active GPU as a proxy for the DDC communication. Which method gets used
depends on the order in which the drivers initialize, the inactive GPU
will happily use whatever is available and it automatically receives
a hotplug event once either method becomes ready for use.
But, once again, the patches implementing proxying (patches 18 - 22)
are still in a state of flux and not ready for prime time, unlike the
prior ones which seem stable. Folks are hereby invited to poke holes
into them and I'm looking forward to your feedback.
Thanks,
Lukas
Dave Airlie (1):
vga_switcheroo: Lock/unlock DDC lines
Lukas Wunner (15):
vga_switcheroo: Lock/unlock DDC lines harder
Revert "vga_switcheroo: Add helper function to get the active
client"
Revert "vga_switcheroo: add reprobe hook for fbcon to recheck
connected outputs."
drm/nouveau: Lock/unlock DDC lines on probe
vga_switcheroo: Generate hotplug event on handler and proxy
registration
drm/i915: Preserve SSC earlier
drm/i915: Reprobe eDP and LVDS connectors on hotplug event
drm/i915: On fb alloc failure, unref gem object where it gets refed
drm: Create new fb and replace default 1024x768 fb on hotplug event
drm/nouveau/timer: Fall back to kernel timer if GPU timer read failed
vga_switcheroo: Allow using active client as proxy when reading
DDC/AUX
drm: Amend struct drm_dp_aux with connector attribute
drm: Use vga_switcheroo active client as proxy when reading DDC/AUX
drm/nouveau/i2c: Use vga_switcheroo active client as proxy when
reading DDC/AUX
drm/nouveau: Use vga_switcheroo active client as proxy when probing
DDC on LVDS
Matthew Garrett (1):
apple-gmux: Assign apple_gmux_data before registering
Seth Forshee (4):
vga_switcheroo: Add support for switching only the DDC
vga_switcheroo: Add helper function to get the active client
apple-gmux: Add switch_ddc support
drm/edid: Switch DDC when reading the EDID
Tvrtko Ursulin (1):
drm/i915: Fix failure paths around initial fbdev allocation
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 -
drivers/gpu/drm/drm_dp_helper.c | 14 ++
drivers/gpu/drm/drm_edid.c | 23 ++-
drivers/gpu/drm/drm_fb_helper.c | 41 ++++-
drivers/gpu/drm/i915/i915_dma.c | 1 -
drivers/gpu/drm/i915/intel_display.c | 91 +++++++---
drivers/gpu/drm/i915/intel_dp.c | 39 +++--
drivers/gpu/drm/i915/intel_drv.h | 6 +
drivers/gpu/drm/i915/intel_fbdev.c | 46 ++---
drivers/gpu/drm/i915/intel_lvds.c | 67 ++++---
drivers/gpu/drm/i915/intel_panel.c | 4 +-
drivers/gpu/drm/msm/edp/edp_connector.c | 2 +
drivers/gpu/drm/nouveau/include/nvkm/subdev/i2c.h | 1 +
drivers/gpu/drm/nouveau/nouveau_connector.c | 18 +-
drivers/gpu/drm/nouveau/nouveau_dp.c | 20 +++
drivers/gpu/drm/nouveau/nouveau_fbcon.c | 10 +-
drivers/gpu/drm/nouveau/nouveau_vga.c | 8 -
drivers/gpu/drm/nouveau/nvkm/engine/disp/dport.c | 6 +-
drivers/gpu/drm/nouveau/nvkm/engine/disp/outpdp.c | 2 +-
drivers/gpu/drm/nouveau/nvkm/engine/disp/outpdp.h | 1 +
drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c | 24 +++
drivers/gpu/drm/nouveau/nvkm/subdev/timer/gk20a.c | 4 +
drivers/gpu/drm/nouveau/nvkm/subdev/timer/nv04.c | 9 +
drivers/gpu/drm/nouveau/nvkm/subdev/timer/nv04.h | 1 +
drivers/gpu/drm/radeon/atombios_dp.c | 1 +
drivers/gpu/drm/radeon/radeon_device.c | 1 -
drivers/gpu/drm/radeon/radeon_fb.c | 11 +-
drivers/gpu/drm/tegra/sor.c | 1 +
drivers/gpu/vga/vga_switcheroo.c | 204 +++++++++++++++++++++-
drivers/platform/x86/apple-gmux.c | 35 +++-
include/drm/drm_dp_helper.h | 5 +
include/linux/vga_switcheroo.h | 18 +-
32 files changed, 590 insertions(+), 125 deletions(-)
--
1.8.5.2 (Apple Git-48)
Daniel Vetter
2015-Aug-12 14:16 UTC
[Nouveau] [Intel-gfx] [PATCH v2 00/22] Enable gpu switching on the MacBook Pro
On Tue, Aug 11, 2015 at 12:29:17PM +0200, Lukas Wunner wrote:> This is a follow-up to the v1 posted in April: > http://lists.freedesktop.org/archives/dri-devel/2015-April/081515.html > > > Patches 1 - 17 enable GPU switching on the pre-retina MacBook Pro. > These were tested successfully by multiple people and solve two > tickets in Bugzilla: > https://bugzilla.kernel.org/show_bug.cgi?id=88861 > https://bugs.freedesktop.org/show_bug.cgi?id=61115 > > Patches 18 - 22 are a preview of how we're tackling retina support. > Those are marked experimental and are NOT ready to be merged yet. > Feedback on them is welcome. > > The patches are based on drm-next. > > They were tested on the following hardware (thanks a lot everyone!): > Tested-by: Paul Hordiienko <pvt.gord at gmail.com> > [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] > Tested-by: William Brown <william at blackhats.net.au> > [MBP 8,2 2011 intel SNB + amd turks pre-retina] > Tested-by: Lukas Wunner <lukas at wunner.de> > [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] > Tested-by: Bruno Bierbaumer <bruno at bierbaumer.net> > [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress] > > > What's new: > > * By default the MBP boots with the display switched to the discrete GPU > but it can be forced to the integrated GPU with an EFI boot variable. > Here's a handy tool for that: https://github.com/0xbb/gpu-switch > v1 didn't work in this configuration, v2 does. > > * Reprobing if the inactive GPU initializes before the apple-gmux module: > v1 used Matthew Garrett's approach of adding a driver callback. > v2 simply generates a hotplug event instead. nouveau polls its outputs > every 10 seconds so we want it to poll immediately once apple-gmux > registers. That is achieved by the hotplug event. The i915 driver is > changed to behave identically to nouveau. (Right now it deletes LVDS > and eDP connectors from the mode configuration if they can't be probed, > deeming them to be ghosts.)I thought -EDEFERREDPROBE is what we should be using if drivers don't get loaded in the right order? Hand-rolling depency avoidance stuff is imo a horrible idea.> * Framebuffer recreation if the inactive GPU initializes before the > apple-gmux module (i.e. discarding the default 1024x768 fb and replacing > with a new one with the actual panel resolution): v1 only supported this > for i915, v2 has a generic solution which works with nouveau and radeon > as well. (Necessary if the discrete GPU is forced to be the inactive one > on boot via the EFI variable.)Would completely remove the need for this ;-)> * Generally lots of rough edges were smoothed to hopefully make the > patches more suitable for merging. E.g. there's a bug in i915 where > the SSC status set by BIOS is preserved too late and v1 only contained > a workaround, whereas v2 contains a proper fix in a separate commit. > > > The long journey towards retina support: > > The pixel clock required for retina resolution is not supported by LVDS > (which was used on pre-retinas), necessitating eDP instead. Problem is, > the gmux register which switches the DDC lines on pre-retinas doesn't > switch the AUX channel on retinas. Disassembling the OS X driver revealed > that the gmux in retina MBPs gained an additional register 0x7f which gets > written to when setting up the eDP configuration. There was some hope that > this might switch the AUX channel. Alas, we tried writing various values > to that register but were unable to get the inactive GPU to talk to the > panel. The purpose of register 0x7f remains a mystery. > > Teardowns of the first generation retina MBP name the NXP CBTL06142 and > TI HD3SS212 as multiplexers and according to the data sheets I've found, > neither supports switching the AUX channel separately from the main link. > > Matthew Garrett had the idea of having the active GPU stash the EDID and > the first 8 bytes of the DPCD (receiver capabilities) and letting the > inactive GPU retrieve that data. I rebased and rewrote his patches and > got everything working, only to discover that the drivers are unhappy > with just 8 bytes of DPCD. They need full access to the DPCD to set up > their outputs. We could stash the entire DPCD but some parts of it are > mutable so the stashed data may become stale when the active GPU performs > writes to the DPCD. > > So I had the idea of using the active GPU as a proxy to talk to the panel, > thus emulating switching of the AUX channel in software. We can leverage > the struct drm_dp_aux and i2c_adapter (for DDC) to achieve this, swapping > the inactive GPU's structs with those of the active GPU on the fly. > That approach is implemented in patches 18 - 22 but there are still some > driver issues that I'm debugging. The current status as per the latest > logs Bruno sent me is that i915 rejects the mode retrieved via proxying > with CLOCK_HIGH and nouveau aborts link training halfway through. > Bottom line is that it's not yet working but we're getting closer. > > As a side effect, the pre-retinas gain a second way to initialize their > outputs: They can either use gmux to switch the DDC lines, or use the > active GPU as a proxy for the DDC communication. Which method gets used > depends on the order in which the drivers initialize, the inactive GPU > will happily use whatever is available and it automatically receives > a hotplug event once either method becomes ready for use. > > But, once again, the patches implementing proxying (patches 18 - 22) > are still in a state of flux and not ready for prime time, unlike the > prior ones which seem stable. Folks are hereby invited to poke holes > into them and I'm looking forward to your feedback.You can't share the dp aux like that. It's true that we need a bit more data (there's a few eDP related feature blocsk we need), but sharing the aux channel entirely is no-go. If you do you get drivers trying to link train and at best this fails and at worst you'll upset the configuration of the other driver and piss of the panel enough to need a hard reset until it works again. I think just reading edid and the relevant dp aux data in apple-gmux or somewhere like that and stalling driver load until that's ready is the only clean option. And of course we must ensure that inactive drivers don't try to use the epd link at all since that will piss off the panel. I think the real tricky bit here with vgaswitcheroo is locking, I need to take a separate lock at the patches for that. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Lukas Wunner
2015-Aug-12 23:37 UTC
[Nouveau] [Intel-gfx] [PATCH v2 00/22] Enable gpu switching on the MacBook Pro
Hi Daniel, On Wed, Aug 12, 2015 at 04:16:25PM +0200, Daniel Vetter wrote:> > * Reprobing if the inactive GPU initializes before the apple-gmux module: > > v1 used Matthew Garrett's approach of adding a driver callback. > > v2 simply generates a hotplug event instead. nouveau polls its outputs > > every 10 seconds so we want it to poll immediately once apple-gmux > > registers. That is achieved by the hotplug event. The i915 driver is > > changed to behave identically to nouveau. (Right now it deletes LVDS > > and eDP connectors from the mode configuration if they can't be probed, > > deeming them to be ghosts.) > > I thought -EDEFERREDPROBE is what we should be using if drivers don't get > loaded in the right order? Hand-rolling depency avoidance stuff is imo a > horrible idea.[...]> I think just reading edid and the relevant dp aux data in apple-gmux or > somewhere like that and stalling driver load until that's ready is the > only clean option.I'm afraid we can't stall initialization of a driver like that because even though the GPU may not be switched to the panel, it may still have an external monitor attached. All MacBook Pros have external DP and/or HDMI ports and these are either soldered to the discrete GPU (model year 2011 and onwards) or switchable between the discrete and integrated GPU (until 2010; I think they are even switchable separately from the panel). So basically we'd have to initialize the driver normally, and if intel_lvds_init() or intel_edp_init_connector() fail we'd have to somehow pass that up the call chain so that i915_pci_probe() can return -EPROBE_DEFER. And whenever we're asked to reprobe we'd repeat initialization of the LVDS or eDP connector. I'm wondering what the benefit is compared to just keeping the connector in the mode configuration, but with status disconnected, and reprobing it when the ->output_poll_changed callback gets invoked? Because that's what nouveau already does, and what I've changed i915 to do with patch 13. vga_switcheroo calls drm_kms_helper_hotplug_event() when the handler registers (patch 11), which will invoke ->output_poll_changed. So we're talking about the Official DRM Callback [tm] to probe outputs, not "hand-rolling depency avoidance". :-)> > * Framebuffer recreation if the inactive GPU initializes before the > > apple-gmux module (i.e. discarding the default 1024x768 fb and replacing > > with a new one with the actual panel resolution): v1 only supported this > > for i915, v2 has a generic solution which works with nouveau and radeon > > as well. (Necessary if the discrete GPU is forced to be the inactive one > > on boot via the EFI variable.) > > Would completely remove the need for this ;-)Unfortunately not: We'd still have to initialize the driver to be able to drive external displays. If there are initially no connectors with modes, we'll once again end up with the 1024x768 fb.> You can't share the dp aux like that. It's true that we need a bit more > data (there's a few eDP related feature blocsk we need), but sharing the > aux channel entirely is no-go. If you do you get drivers trying to link > train and at best this fails and at worst you'll upset the configuration > of the other driver and piss of the panel enough to need a hard reset > until it works again.Yes, so far proxying of the AUX channel is read-only. In those cases when writing is necessary for setting up the output, I'm adding code to check if the current content of the DPCD is identical to what's being written and if so, skip the write. We'll see if this stategy is sufficient for the drivers to set up their outputs.> I think the real tricky bit here with vgaswitcheroo is locking, I need to > take a separate lock at the patches for that.Locking when switching only the DDC lines is facilitated by the ddc_lock attribute of struct vgasr_priv. This is all local to vga_switcheroo.c and contained in patches 5 and 6. Locking when proxying the AUX channel is facilitated by the hw_mutex attribute of struct drm_dp_aux. nouveau has its own locking mechanism contained in drivers/gpu/drm/nouveau/nvkm/subdev/i2c/pad*.c. Thus, when proxying via nouveau, there are two locking mechanisms at work (drm_dp_aux hw_mutex as outer lock + pad as inner lock). This is nothing introduced by this patch set, all existing code. Locking of access to the struct vgasr_priv is facilitated by the vgasr_mutex in vga_switcheroo.c. Also existing code. Best regards, Lukas
Possibly Parallel Threads
- [PATCH v5 00/12] Enable GPU switching on pre-retina MacBook Pro
- [PATCH v5 00/12] Enable GPU switching on pre-retina MacBook Pro
- [Intel-gfx] [PATCH v2 00/22] Enable gpu switching on the MacBook Pro
- [PATCH v3 0/6] Enable gpu switching on the MacBook Pro
- [PATCH v5 00/12] Enable GPU switching on pre-retina MacBook Pro