Re: [PATCH v2 07/22] Revert "vga_switcheroo: Add helper function to get the active client"

From: Daniel Vetter <daniel@ffwll.ch>
To: Lukas Wunner <lukas@wunner.de>
Cc: Andreas Heider <andreas@meetr.de>,
	Paul Hordiienko <pvt.gord@gmail.com>,
	William Brown <william@blackhats.net.au>,
	dri-devel@lists.freedesktop.org,
	Bruno Bierbaumer <bruno@bierbaumer.net>,
	Matthew Garrett <mjg59@coreos.com>,
	Dave Airlie <airlied@redhat.com>
Subject: Re: [PATCH v2 07/22] Revert "vga_switcheroo: Add helper function to get the active client"
Date: Wed, 12 Aug 2015 23:10:59 +0200	[thread overview]
Message-ID: <20150812211059.GS17734@phenom.ffwll.local> (raw)
In-Reply-To: <20150812173432.GA5965@wunner.de>

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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel