Hi, Le 13/03/2017 à 19:49, Jose Abreu a écrit : > Hi Russell, > > > On 13-03-2017 13:10, Russell King - ARM Linux wrote: >> On Mon, Mar 13, 2017 at 12:55:58PM +0000, Jose Abreu wrote: >>> Hi, >>> >>> >>> On 13-03-2017 09:36, Russell King - ARM Linux wrote: >>>> On Mon, Mar 13, 2017 at 10:27:08AM +0100, Neil Armstrong wrote: >>>>> On 03/10/2017 10:35 AM, Romain Perier wrote: >>>>>> Currently, the audio sampler clock is enabled from dw_hdmi_setup() at >>>>>> step E. and is kept enabled for later use. This clock should be enabled >>>>>> and disabled along with the actual audio stream and not always on (that >>>>>> is bad for PM). Futhermore, this might cause sound glitches with some >>>>>> HDMI devices, as the CTS+N is forced to zero when the stream is disabled >>>>>> while the audio clock is still running. >>>>>> >>>>>> This commit adds a parameter to hdmi_audio_enable_clk() that controls >>>>>> when the audio sample clock must be enabled or disabled. Then, it moves >>>>>> the call to this function into dw_hdmi_audio_enable() and >>>>>> dw_hdmi_audio_disable(). >>>>>> >>>>>> Signed-off-by: Romain Perier <romain.perier@xxxxxxxxxxxxx> >>>>>> --- >>>>>> drivers/gpu/drm/bridge/dw-hdmi.c | 15 +++++++++------ >>>>>> 1 file changed, 9 insertions(+), 6 deletions(-) >>>>>> >>>>> Hi Romain, Russell, Jose, >>>>> >>>>> This is a little out of scope, but I was wondering why the CTS calculation >>>>> was not left in AUTO mode in the dw-hdmi driver ? >>>> There is no indication in the iMX6 manuals that the iMX6 supports >>>> automatic CTS calculation. Bits 7:4 of the AUD_CTS3 register are >>>> marked as "reserved". >>>> >>>> We're reliant on the information in the iMX6 manuals as we don't have >>>> access to Synopsis' databooks for these parts (I understand you have >>>> to be a customer to have access to that.) >>>> >>> (Synopsis -> Synopsys :)) >>> >>> Trying to catch up with the conversation: >>> >>> 1) In AHB audio mode the bits are always reserved. >>> 2) I think we should enable/disable clock instead of just forcing >>> N/CTS, though, I don't know what could be the implications for >>> iMX platform. I remember at the time I tested this using I2S >>> (I've never used AHB), and HDMI protocol analyzers were >>> complaining about the N/CTS being forced to zero. >> What you're recommending is to basically ignore the published workaround, >> which, presumably, was arrived at by proper analysis of the issue both >> by Freescale engineers and Synopsys engineers, and instead try some other >> solution. >> >> I'm not sure that's a good idea. Only those with knowledge of the IP can >> say what effect disabling the audio clock will have: will it still allow >> the FIFO to be loaded, as required by the errata? If not, it's not a >> solution that AHB can use. I would imagine that _if_ it was as simple as >> disabling the audio clock, that simple approach would have been documented >> in Freescale's errata documents, rather than the rather more complex >> method of zeroing the CTS/N values. > I'm just following what the user guide says: the final step of > configuration is enabling the audio clock. There is no reference > neither in datasheet neither in user guide about this behavior > but, as I said, I've never used AHB so I can't prove what is the > best solution. Could it be platform specific? And that's precisely why I am enabling it > >> I think there are two choices here: >> >> 1) get some definitive information about the IP and the errata for AHB, >> and determine whether the solution you propose would work. (That's >> out of scope for me, I've no contacts with FSL/NXP and I'm not a >> Synopsys customer.) > This is the right thing to do but if you can't test in the > Freescale platform then we have not much else to do. > > Best regards, > Jose Miguel Abreu > >> 2) the I2S audio support stops re-using the AHB audio support functions, >> switching to their own implementation that behaves correctly for them. >> (Remember, it was I2S's choice to re-use the AHB audio support functions >> I added which we're now discussing - they didn't have to do that, and >> regressing AHB audio just for I2S is not something we should ever >> consider doing.) >> The workaround looks AHB specific in any cases and does not seem to work with I2S. The I2S variant should not used the same functions, at least for enabling/disabling audio stream. Regards, Romain _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
