[1/4] drm/i915: Add audio pin sense / ELD callback
diff mbox

Message ID 1439974138-3296-2-git-send-email-david.henningsson@canonical.com
State New
Headers show

Commit Message

David Henningsson Aug. 19, 2015, 8:48 a.m. UTC
This callback will be called by the i915 driver to notify the hda
driver that its HDMI information needs to be refreshed, i e,
that audio output is now available (or unavailable) - usually as a
result of a monitor being plugged in (or unplugged).

Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 include/drm/i915_component.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Daniel Vetter Aug. 26, 2015, 8:33 a.m. UTC | #1
On Wed, Aug 19, 2015 at 10:48:55AM +0200, David Henningsson wrote:
> This callback will be called by the i915 driver to notify the hda
> driver that its HDMI information needs to be refreshed, i e,
> that audio output is now available (or unavailable) - usually as a
> result of a monitor being plugged in (or unplugged).
> 
> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
> ---
>  include/drm/i915_component.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
> index c9a8b64..ab5bde37 100644
> --- a/include/drm/i915_component.h
> +++ b/include/drm/i915_component.h
> @@ -34,6 +34,18 @@ struct i915_audio_component {
>  		void (*codec_wake_override)(struct device *, bool enable);
>  		int (*get_cdclk_freq)(struct device *);
>  	} *ops;
> +
> +	const struct i915_audio_component_audio_ops {
> +		void *audio_ptr;
> +		/**
> +		 * Call from i915 driver, notifying the HDA driver that
> +		 * pin sense and/or ELD information has changed.
> +		 * @audio_ptr:		HDA driver object
> +		 * @port:		Which port has changed (PORTA / PORTB / PORTC etc)
> +		 * @port_mst_index:	Index within that port, for DisplayPort multistreaming
> +		 */
> +		void (*pin_eld_notify)(void *audio_ptr, int port, int port_mst_index);
> +	} *audio_ops;

This won't work as proper kerneldoc, but you get away with it since it's
not pulled into the drm.tmpl. See my comments for the new set_audio_rate
callback.
-Daniel

>  };
>  
>  #endif /* _I915_COMPONENT_H_ */
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
David Henningsson Aug. 26, 2015, 6:39 p.m. UTC | #2
On 2015-08-26 10:33, Daniel Vetter wrote:
> On Wed, Aug 19, 2015 at 10:48:55AM +0200, David Henningsson wrote:
>> This callback will be called by the i915 driver to notify the hda
>> driver that its HDMI information needs to be refreshed, i e,
>> that audio output is now available (or unavailable) - usually as a
>> result of a monitor being plugged in (or unplugged).
>>
>> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
>> ---
>>   include/drm/i915_component.h | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
>> index c9a8b64..ab5bde37 100644
>> --- a/include/drm/i915_component.h
>> +++ b/include/drm/i915_component.h
>> @@ -34,6 +34,18 @@ struct i915_audio_component {
>>   		void (*codec_wake_override)(struct device *, bool enable);
>>   		int (*get_cdclk_freq)(struct device *);
>>   	} *ops;
>> +
>> +	const struct i915_audio_component_audio_ops {
>> +		void *audio_ptr;
>> +		/**
>> +		 * Call from i915 driver, notifying the HDA driver that
>> +		 * pin sense and/or ELD information has changed.
>> +		 * @audio_ptr:		HDA driver object
>> +		 * @port:		Which port has changed (PORTA / PORTB / PORTC etc)
>> +		 * @port_mst_index:	Index within that port, for DisplayPort multistreaming
>> +		 */
>> +		void (*pin_eld_notify)(void *audio_ptr, int port, int port_mst_index);
>> +	} *audio_ops;
>
> This won't work as proper kerneldoc, but you get away with it since it's
> not pulled into the drm.tmpl. See my comments for the new set_audio_rate
> callback.

Sorry, my google failed me, so I can't find your comments for the 
set_audio_rate callback.

Apart from the kerneldoc issue, are you okay with acking the patch, at 
least the first two i915 ones, and agree with Takashi which tree this 
should go through?
Jani Nikula Aug. 27, 2015, 2:24 p.m. UTC | #3
On Wed, 26 Aug 2015, David Henningsson <david.henningsson@canonical.com> wrote:
> On 2015-08-26 10:33, Daniel Vetter wrote:
>> On Wed, Aug 19, 2015 at 10:48:55AM +0200, David Henningsson wrote:
>>> This callback will be called by the i915 driver to notify the hda
>>> driver that its HDMI information needs to be refreshed, i e,
>>> that audio output is now available (or unavailable) - usually as a
>>> result of a monitor being plugged in (or unplugged).
>>>
>>> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
>>> ---
>>>   include/drm/i915_component.h | 12 ++++++++++++
>>>   1 file changed, 12 insertions(+)
>>>
>>> diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
>>> index c9a8b64..ab5bde37 100644
>>> --- a/include/drm/i915_component.h
>>> +++ b/include/drm/i915_component.h
>>> @@ -34,6 +34,18 @@ struct i915_audio_component {
>>>   		void (*codec_wake_override)(struct device *, bool enable);
>>>   		int (*get_cdclk_freq)(struct device *);
>>>   	} *ops;
>>> +
>>> +	const struct i915_audio_component_audio_ops {
>>> +		void *audio_ptr;
>>> +		/**
>>> +		 * Call from i915 driver, notifying the HDA driver that
>>> +		 * pin sense and/or ELD information has changed.
>>> +		 * @audio_ptr:		HDA driver object
>>> +		 * @port:		Which port has changed (PORTA / PORTB / PORTC etc)
>>> +		 * @port_mst_index:	Index within that port, for DisplayPort multistreaming
>>> +		 */
>>> +		void (*pin_eld_notify)(void *audio_ptr, int port, int port_mst_index);
>>> +	} *audio_ops;
>>
>> This won't work as proper kerneldoc, but you get away with it since it's
>> not pulled into the drm.tmpl. See my comments for the new set_audio_rate
>> callback.
>
> Sorry, my google failed me, so I can't find your comments for the 
> set_audio_rate callback.

It's on the related thread [1], specifically the subthread starting at
[2]. I guess there's no direct overlap between the two series, but it
would be helpful if you can look at each other's work so there's no
surprises.

BR,
Jani.


[1] http://mid.gmane.org/1439880714-40931-1-git-send-email-libin.yang@intel.com
[2] http://mid.gmane.org/20150826081735.GZ20434@phenom.ffwll.local

>
> Apart from the kerneldoc issue, are you okay with acking the patch, at 
> least the first two i915 ones, and agree with Takashi which tree this 
> should go through?
>
> -- 
> David Henningsson, Canonical Ltd.
> https://launchpad.net/~diwic
Jani Nikula Aug. 28, 2015, 12:55 p.m. UTC | #4
On Wed, 19 Aug 2015, David Henningsson <david.henningsson@canonical.com> wrote:
> This callback will be called by the i915 driver to notify the hda
> driver that its HDMI information needs to be refreshed, i e,
> that audio output is now available (or unavailable) - usually as a
> result of a monitor being plugged in (or unplugged).
>
> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
> ---
>  include/drm/i915_component.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
> index c9a8b64..ab5bde37 100644
> --- a/include/drm/i915_component.h
> +++ b/include/drm/i915_component.h
> @@ -34,6 +34,18 @@ struct i915_audio_component {
>  		void (*codec_wake_override)(struct device *, bool enable);
>  		int (*get_cdclk_freq)(struct device *);
>  	} *ops;
> +
> +	const struct i915_audio_component_audio_ops {
> +		void *audio_ptr;
> +		/**
> +		 * Call from i915 driver, notifying the HDA driver that
> +		 * pin sense and/or ELD information has changed.
> +		 * @audio_ptr:		HDA driver object
> +		 * @port:		Which port has changed (PORTA / PORTB / PORTC etc)
> +		 * @port_mst_index:	Index within that port, for DisplayPort multistreaming
> +		 */
> +		void (*pin_eld_notify)(void *audio_ptr, int port, int port_mst_index);

In the HDMI sample rate notification series [1] I explicitly asked Libin
to drop a similar port_mst_index parameter since we don't use it yet,
and we are not sure what to stick there yet. Indeed not having it now
enforces us to change the API and check for each call site when enabling
MST audio.

Other than that,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>



[1] http://mid.gmane.org/1439880714-40931-1-git-send-email-libin.yang@intel.com

> +	} *audio_ops;
>  };
>  
>  #endif /* _I915_COMPONENT_H_ */
> -- 
> 1.9.1
>

Patch
diff mbox

diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
index c9a8b64..ab5bde37 100644
--- a/include/drm/i915_component.h
+++ b/include/drm/i915_component.h
@@ -34,6 +34,18 @@  struct i915_audio_component {
 		void (*codec_wake_override)(struct device *, bool enable);
 		int (*get_cdclk_freq)(struct device *);
 	} *ops;
+
+	const struct i915_audio_component_audio_ops {
+		void *audio_ptr;
+		/**
+		 * Call from i915 driver, notifying the HDA driver that
+		 * pin sense and/or ELD information has changed.
+		 * @audio_ptr:		HDA driver object
+		 * @port:		Which port has changed (PORTA / PORTB / PORTC etc)
+		 * @port_mst_index:	Index within that port, for DisplayPort multistreaming
+		 */
+		void (*pin_eld_notify)(void *audio_ptr, int port, int port_mst_index);
+	} *audio_ops;
 };
 
 #endif /* _I915_COMPONENT_H_ */