On Thu, Sep 24, 2015 at 11:40:52AM +0200, Takashi Iwai wrote:
> On Wed, 23 Sep 2015 09:13:28 +0200,
> Daniel Vetter wrote:
> >
> > On Thu, Aug 27, 2015 at 04:43:43PM +0200, Lukas Wunner wrote:
> > > The active attribute in struct vga_switcheroo_client denotes whether
> > > the outputs are currently switched to this client. The attribute is
> > > only meaningful for vga clients. It is never used for audio clients.
> > >
> > > The function vga_switcheroo_register_audio_client() misuses this
> > > attribute to store whether the audio device is fully initialized.
> > > Most likely there was a misunderstanding about the meaning of
> > > "active" when this was added.
> > >
> > > Set the active attribute to false for audio clients. Remove the
> > > active parameter from vga_switcheroo_register_audio_client() and
> > > its sole caller, hda_intel.c:register_vga_switcheroo().
> > >
> > > vga_switcheroo_register_audio_client() was introduced by 3e9e63dbd374
> > > ("vga_switcheroo: Add the support for audio clients"). Its use in
> > > hda_intel.c was introduced by a82d51ed24bb ("ALSA: hda - Support
> > > VGA-switcheroo").
> > >
> > > v1.1: The changes above imply that in find_active_client() the call
> > > to client_is_vga() is now superfluous. Drop it.
> > >
> > > Cc: Takashi Iwai <tiwai@xxxxxxx>
> > > Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
> >
> > Takashi, can you pls ack this for merging through drm-misc? Patch lgtm.
>
> For the code change, feel free to take my ack:
> Reviewed-by: Takashi Iwai <tiwai@xxxxxxx>
>
> But it'd be better if the changelog can be rephrased a bit.
I added your clarification to the commit message and pulled in the patch.
Thanks, Daniel
>
>
> thanks,
>
> Takashi
>
> > -Daniel
> >
> > > ---
> > > drivers/gpu/vga/vga_switcheroo.c | 7 +++----
> > > include/linux/vga_switcheroo.h | 4 ++--
> > > sound/pci/hda/hda_intel.c | 3 +--
> > > 3 files changed, 6 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
> > > index 2e7e2f8e..563b82f 100644
> > > --- a/drivers/gpu/vga/vga_switcheroo.c
> > > +++ b/drivers/gpu/vga/vga_switcheroo.c
> > > @@ -290,7 +290,6 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
> > > * @pdev: client pci device
> > > * @ops: client callbacks
> > > * @id: client identifier, see enum vga_switcheroo_client_id
> > > - * @active: whether the audio device is fully initialized
> > > *
> > > * Register audio client (audio device on a GPU). The power state of the
> > > * client is assumed to be ON.
> > > @@ -299,9 +298,9 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
> > > */
> > > int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> > > const struct vga_switcheroo_client_ops *ops,
> > > - int id, bool active)
> > > + int id)
> > > {
> > > - return register_client(pdev, ops, id | ID_BIT_AUDIO, active, false);
> > > + return register_client(pdev, ops, id | ID_BIT_AUDIO, false, false);
> > > }
> > > EXPORT_SYMBOL(vga_switcheroo_register_audio_client);
> > >
> > > @@ -333,7 +332,7 @@ find_active_client(struct list_head *head)
> > > struct vga_switcheroo_client *client;
> > >
> > > list_for_each_entry(client, head, list)
> > > - if (client->active && client_is_vga(client))
> > > + if (client->active)
> > > return client;
> > > return NULL;
> > > }
> > > diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
> > > index fe90bfc..3764991 100644
> > > --- a/include/linux/vga_switcheroo.h
> > > +++ b/include/linux/vga_switcheroo.h
> > > @@ -128,7 +128,7 @@ int vga_switcheroo_register_client(struct pci_dev *dev,
> > > bool driver_power_control);
> > > int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> > > const struct vga_switcheroo_client_ops *ops,
> > > - int id, bool active);
> > > + int id);
> > >
> > > void vga_switcheroo_client_fb_set(struct pci_dev *dev,
> > > struct fb_info *info);
> > > @@ -154,7 +154,7 @@ static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_i
> > > static inline int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { return 0; }
> > > static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
> > > const struct vga_switcheroo_client_ops *ops,
> > > - int id, bool active) { return 0; }
> > > + int id) { return 0; }
> > > static inline void vga_switcheroo_unregister_handler(void) {}
> > > static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
> > > static inline int vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; }
> > > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> > > index c38c68f..e819013 100644
> > > --- a/sound/pci/hda/hda_intel.c
> > > +++ b/sound/pci/hda/hda_intel.c
> > > @@ -1143,8 +1143,7 @@ static int register_vga_switcheroo(struct azx *chip)
> > > * is there any machine with two switchable HDMI audio controllers?
> > > */
> > > err = vga_switcheroo_register_audio_client(chip->pci, &azx_vs_ops,
> > > - VGA_SWITCHEROO_DIS,
> > > - hda->probe_continued);
> > > + VGA_SWITCHEROO_DIS);
> > > if (err < 0)
> > > return err;
> > > hda->vga_switcheroo_registered = 1;
> > > --
> > > 1.8.5.2 (Apple Git-48)
> > >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
