On 2015-07-22 10:22, Takashi Iwai wrote:
On Tue, 21 Jul 2015 09:57:24 +0200,
David Henningsson wrote:
This struct will be used to transfer information from the i915
driver to the hda driver on HDMI hotplug events.
Signed-off-by: David Henningsson <david.henningsson@xxxxxxxxxxxxx>
Looks good to me, just a few nitpicking:
---
include/drm/i915_component.h | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
index c9a8b64..4fc0db3 100644
--- a/include/drm/i915_component.h
+++ b/include/drm/i915_component.h
@@ -24,8 +24,22 @@
#ifndef _I915_COMPONENT_H_
#define _I915_COMPONENT_H_
+struct hdac_bus;
+
+struct i915_audio_hotplug_info {
+ int connector_type; /* DRM_MODE_CONNECTOR_*, meant for userspace */
+ int connector_type_id; /* Index within a DRM_MODE_CONNECTOR_* type, meant for userspace */
+ int port; /* Used for mapping to affected nid */
+ int port_multi_stream_device; /* For DP multi-streaming */
+
+ bool plugged_in;
+ uint8_t *eld;
Use u8 or just unsigned char as it's a in-kernel API.
Also, safer to add const, since this is read-only for audio side.
Ok.
+ int eld_size;
+};
+
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).
const struct i915_audio_component_ops {
struct module *owner;
@@ -34,6 +48,11 @@ struct i915_audio_component {
void (*codec_wake_override)(struct device *, bool enable);
int (*get_cdclk_freq)(struct device *);
} *ops;
+
+ const struct i915_audio_component_cb_ops {
+ struct module *owner;
Do we need the owner field at all?
It was merely for symmetry. I'll remove it for v2.
+ void (*hotplug_notify)(struct hdac_bus *, const struct i915_audio_hotplug_info *);
+ } *cb_ops;
cb_ops doesn't sound intuitive. Any better name?
I was thinking of it as "callback ops", i e, calls that go in the
reverse direction compared to the already existing "ops".
But if we call the device "audio_dev" as you suggested above, then maybe
"audio_ops" would be nice and symmetric?
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
