| Message ID | f081b2513e26998c6f994305564e4a10e65ca820.1439288957.git.lukas@wunner.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 21, 2015 at 10:39:45AM +0200, Lukas Wunner wrote: > This reverts commit 26814ce68904c9faf977c90edac798156311981f. > > The helper function is no longer needed after Dave Airlie's rewrite > of vga_switcheroo_switch_ddc(), the commit introducing it was only > included because 31f23c3d488e ("drm/edid: Switch DDC when reading > the EDID") does not compile without it. > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115 > Tested-by: Paul Hordiienko <pvt.gord@gmail.com> > [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] > Tested-by: William Brown <william@blackhats.net.au> > [MBP 8,2 2011 intel SNB + amd turks pre-retina] > Tested-by: Lukas Wunner <lukas@wunner.de> > [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] > Tested-by: Bruno Bierbaumer <bruno@bierbaumer.net> > [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress] > > Signed-off-by: Lukas Wunner <lukas@wunner.de> > --- > drivers/gpu/vga/vga_switcheroo.c | 14 -------------- > include/linux/vga_switcheroo.h | 2 -- > 2 files changed, 16 deletions(-) > > diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c > index f02e7fc..f0d5475 100644 > --- a/drivers/gpu/vga/vga_switcheroo.c > +++ b/drivers/gpu/vga/vga_switcheroo.c > @@ -214,20 +214,6 @@ find_active_client(struct list_head *head) > return NULL; > } > > -struct pci_dev *vga_switcheroo_get_active_client(void) > -{ > - struct vga_switcheroo_client *client; > - struct pci_dev *pdev = NULL; > - > - mutex_lock(&vgasr_mutex); > - client = find_active_client(&vgasr_priv.clients); > - if (client) > - pdev = client->pdev; > - mutex_unlock(&vgasr_mutex); > - return pdev; > -} > -EXPORT_SYMBOL(vga_switcheroo_get_active_client); you just added this earlier in this very series. Please reorder/squash patches so that this isn't required. -Daniel > - > int vga_switcheroo_get_client_state(struct pci_dev *pdev) > { > struct vga_switcheroo_client *client; > diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h > index 39b25b0..62f303f 100644 > --- a/include/linux/vga_switcheroo.h > +++ b/include/linux/vga_switcheroo.h > @@ -63,7 +63,6 @@ void vga_switcheroo_unregister_handler(void); > > int vga_switcheroo_process_delayed_switch(void); > > -struct pci_dev *vga_switcheroo_get_active_client(void); > int vga_switcheroo_get_client_state(struct pci_dev *dev); > > void vga_switcheroo_set_dynamic_switch(struct pci_dev *pdev, enum vga_switcheroo_state dynamic); > @@ -85,7 +84,6 @@ static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev, > int id, bool active) { return 0; } > static inline void vga_switcheroo_unregister_handler(void) {} > static inline int vga_switcheroo_process_delayed_switch(void) { return 0; } > -static inline struct pci_dev *vga_switcheroo_get_active_client(void) { return NULL; } > static inline int vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; } > > static inline void vga_switcheroo_set_dynamic_switch(struct pci_dev *pdev, enum vga_switcheroo_state dynamic) {} > -- > 1.8.5.2 (Apple Git-48) > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Daniel, thanks for taking a look at the patch set. On Wed, Aug 12, 2015 at 04:25:52PM +0200, Daniel Vetter wrote: > On Tue, Apr 21, 2015 at 10:39:45AM +0200, Lukas Wunner wrote: > > This reverts commit 26814ce68904c9faf977c90edac798156311981f. > > > > The helper function is no longer needed after Dave Airlie's rewrite > > of vga_switcheroo_switch_ddc(), the commit introducing it was only > > included because 31f23c3d488e ("drm/edid: Switch DDC when reading > > the EDID") does not compile without it. [...] > > --- a/drivers/gpu/vga/vga_switcheroo.c > > +++ b/drivers/gpu/vga/vga_switcheroo.c > > @@ -214,20 +214,6 @@ find_active_client(struct list_head *head) > > return NULL; > > } > > > > -struct pci_dev *vga_switcheroo_get_active_client(void) > > -{ > > - struct vga_switcheroo_client *client; > > - struct pci_dev *pdev = NULL; > > - > > - mutex_lock(&vgasr_mutex); > > - client = find_active_client(&vgasr_priv.clients); > > - if (client) > > - pdev = client->pdev; > > - mutex_unlock(&vgasr_mutex); > > - return pdev; > > -} > > -EXPORT_SYMBOL(vga_switcheroo_get_active_client); > > you just added this earlier in this very series. Please reorder/squash > patches so that this isn't required. I would have to squash patches 2, 4 (by Seth Forshee), 5 (by Dave Airlie), 6 and 7 (mine). The work of two of these authors would only be acknowledged in the commit message and the history how the code evolved over the course of 3 years would not be reflected in the git repo. Are you sure? (y/n) I deliberately didn't squash to preserve authorship and history but if you're forcing me at point blank I'll do it. ;-) Context: Seth Forshee of Canonical came up with patches in 2012 which Dave Airlie didn't like. He rewrote them and left them as a WIP in his git repo where I picked them up. Matthew Garrett posted patches of his own last year but since they were based on Seth Forshee's code, they didn't get merged either. The first MacBooks with dual GPUs were introduced 2008, it's 2015 now and we're still missing support in the mainline kernel. The issue is not so much that GPU switching doesn't work (the screen just turns black) but energy consumption because the discrete GPU is used by default and the integrated GPU isn't turned off. So, machines with huge marketshare + shoddy dual GPU support for years = problem. We need to fix this, hence the patch set. Thanks, Lukas
On Wed, Aug 12, 2015 at 07:34:32PM +0200, Lukas Wunner wrote: > Hi Daniel, > > thanks for taking a look at the patch set. > > On Wed, Aug 12, 2015 at 04:25:52PM +0200, Daniel Vetter wrote: > > On Tue, Apr 21, 2015 at 10:39:45AM +0200, Lukas Wunner wrote: > > > This reverts commit 26814ce68904c9faf977c90edac798156311981f. > > > > > > The helper function is no longer needed after Dave Airlie's rewrite > > > of vga_switcheroo_switch_ddc(), the commit introducing it was only > > > included because 31f23c3d488e ("drm/edid: Switch DDC when reading > > > the EDID") does not compile without it. > [...] > > > --- a/drivers/gpu/vga/vga_switcheroo.c > > > +++ b/drivers/gpu/vga/vga_switcheroo.c > > > @@ -214,20 +214,6 @@ find_active_client(struct list_head *head) > > > return NULL; > > > } > > > > > > -struct pci_dev *vga_switcheroo_get_active_client(void) > > > -{ > > > - struct vga_switcheroo_client *client; > > > - struct pci_dev *pdev = NULL; > > > - > > > - mutex_lock(&vgasr_mutex); > > > - client = find_active_client(&vgasr_priv.clients); > > > - if (client) > > > - pdev = client->pdev; > > > - mutex_unlock(&vgasr_mutex); > > > - return pdev; > > > -} > > > -EXPORT_SYMBOL(vga_switcheroo_get_active_client); > > > > you just added this earlier in this very series. Please reorder/squash > > patches so that this isn't required. > > I would have to squash patches 2, 4 (by Seth Forshee), 5 (by Dave Airlie), > 6 and 7 (mine). The work of two of these authors would only be acknowledged > in the commit message and the history how the code evolved over the course > of 3 years would not be reflected in the git repo. > > Are you sure? (y/n) Yes just squash and mention that the patch is based on work from $list_of_other_authors, plus cc them. There's not much point in acknowledging when people write broken patches ;-) > I deliberately didn't squash to preserve authorship and history but if > you're forcing me at point blank I'll do it. ;-) > > Context: Seth Forshee of Canonical came up with patches in 2012 which Dave > Airlie didn't like. He rewrote them and left them as a WIP in his git repo > where I picked them up. Matthew Garrett posted patches of his own last year > but since they were based on Seth Forshee's code, they didn't get merged > either. > > The first MacBooks with dual GPUs were introduced 2008, it's 2015 now and > we're still missing support in the mainline kernel. The issue is not so > much that GPU switching doesn't work (the screen just turns black) but > energy consumption because the discrete GPU is used by default and the > integrated GPU isn't turned off. > > So, machines with huge marketshare + shoddy dual GPU support for years > = problem. > > We need to fix this, hence the patch set. Apparently not a lot of people bothered yet to polish this, so it can't be that bad. I guess everyone just buys the basic model with intel only. -Daniel
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c index f02e7fc..f0d5475 100644 --- a/drivers/gpu/vga/vga_switcheroo.c +++ b/drivers/gpu/vga/vga_switcheroo.c @@ -214,20 +214,6 @@ find_active_client(struct list_head *head) return NULL; } -struct pci_dev *vga_switcheroo_get_active_client(void) -{ - struct vga_switcheroo_client *client; - struct pci_dev *pdev = NULL; - - mutex_lock(&vgasr_mutex); - client = find_active_client(&vgasr_priv.clients); - if (client) - pdev = client->pdev; - mutex_unlock(&vgasr_mutex); - return pdev; -} -EXPORT_SYMBOL(vga_switcheroo_get_active_client); - int vga_switcheroo_get_client_state(struct pci_dev *pdev) { struct vga_switcheroo_client *client; diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h index 39b25b0..62f303f 100644 --- a/include/linux/vga_switcheroo.h +++ b/include/linux/vga_switcheroo.h @@ -63,7 +63,6 @@ void vga_switcheroo_unregister_handler(void); int vga_switcheroo_process_delayed_switch(void); -struct pci_dev *vga_switcheroo_get_active_client(void); int vga_switcheroo_get_client_state(struct pci_dev *dev); void vga_switcheroo_set_dynamic_switch(struct pci_dev *pdev, enum vga_switcheroo_state dynamic); @@ -85,7 +84,6 @@ static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev, int id, bool active) { return 0; } static inline void vga_switcheroo_unregister_handler(void) {} static inline int vga_switcheroo_process_delayed_switch(void) { return 0; } -static inline struct pci_dev *vga_switcheroo_get_active_client(void) { return NULL; } static inline int vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; } static inline void vga_switcheroo_set_dynamic_switch(struct pci_dev *pdev, enum vga_switcheroo_state dynamic) {}
This reverts commit 26814ce68904c9faf977c90edac798156311981f. The helper function is no longer needed after Dave Airlie's rewrite of vga_switcheroo_switch_ddc(), the commit introducing it was only included because 31f23c3d488e ("drm/edid: Switch DDC when reading the EDID") does not compile without it. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115 Tested-by: Paul Hordiienko <pvt.gord@gmail.com> [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] Tested-by: William Brown <william@blackhats.net.au> [MBP 8,2 2011 intel SNB + amd turks pre-retina] Tested-by: Lukas Wunner <lukas@wunner.de> [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Tested-by: Bruno Bierbaumer <bruno@bierbaumer.net> [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress] Signed-off-by: Lukas Wunner <lukas@wunner.de> --- drivers/gpu/vga/vga_switcheroo.c | 14 -------------- include/linux/vga_switcheroo.h | 2 -- 2 files changed, 16 deletions(-)