Re: [PATCH 1/4] drm/i915: Add audio hotplug info struct

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2015-07-22 22:31, Takashi Iwai wrote:
On Wed, 22 Jul 2015 19:52:23 +0200,
David Henningsson wrote:



On 2015-07-22 16:13, Vinod Koul wrote:
On Wed, Jul 22, 2015 at 10:55:55AM +0200, Takashi Iwai wrote:
On Wed, 22 Jul 2015 10:50:03 +0200,
David Henningsson wrote:

    struct i915_audio_component {
    	struct device *dev;
+	struct hdac_bus *hdac_bus;

If we want to be more generic, using a struct device would be better,
e.g.
	struct device *audio_dev;

Does this work? If we want to have the hdac_bus.dev ptr instead of a
hdac_bus ptr, there does not seem to be an obvious way to go from the
audio_dev back to the hdac_bus struct (as snd_hdac_bus_init takes an
arbitrary dev pointer).

Hrm, right, currently it's not straightforward.  Scratch the idea,
then.

That depends on the device we register this with. Actually this makes more
sense to me :)

If we register with struct device *audio_dev, which in this case would be
the codec device we create while probing the bus. This way you are linking i915
ops to the codec device. Ofcourse hdac_device has bus pointer but you can
invoke device callback without even searching for the device :)

It would require some extra setup, so I skipped it (at least for now).

I e, in order to detect codecs, we need to call hdac_i915 functions to
turn the power well on. And in order to connect the codec to the
i915_audio_component, we need to have detected a codec.

Which, now that I think of it, actually gives an interesting potential
race condition, in case the following happens:

1) Monitor is plugged in at boot time
2) i915 initializes
3) hda starts initializing and sets up the audio component
4) i915 finishes initialization and as part of that, calls the hotplug
notify to let hda know that the monitor is plugged in. However, at this
point, hda has not finished initialization yet, so there are no codecs
that listen for this information.

Anyhow, this is not a problem really yet, but it might be if we ever
decide to not write the ELD to the hardware.

For avoid such a problem, maybe we need two ops, one for notification
and one for getting the assigned data.  At the initialization time,
the audio driver queries the assigned status and data.  When a hotplug
happens, it's just notified.  That is, it simply replaces the current
unsol event and the ELD data read via two ops.

I'm about to go on vacation so it would be good to get some closure here. If you both prefer this setup, how about I remove "struct i915_audio_hotplug_info" for now? We will then have:

	const struct i915_audio_component_audio_ops {
		void (*hotplug_notify)(struct hdac_bus *);
	}

...which will make the hda driver read from the hardware. Asking the i915 driver for ELD, connector and hotplug status can then be done in a later patch.

--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux