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.
> + 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;
> 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?
> + void (*hotplug_notify)(struct hdac_bus *, const struct i915_audio_hotplug_info *);
> + } *cb_ops;
cb_ops doesn't sound intuitive. Any better name?
thanks,
Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
