On Wed, Jun 12, 2019 at 11:25:59AM -0400, Sven Van Asbroeck wrote: > On Tue, Jun 11, 2019 at 7:02 AM Russell King <rmk+kernel@xxxxxxxxxxxxxxx> wrote: > > > > Improve the selection of the audio clock divisor so that more modes > > and sample rates work. > > > > Signed-off-by: Russell King <rmk+kernel@xxxxxxxxxxxxxxx> > > --- > > +static u8 tda998x_get_adiv(struct tda998x_priv *priv, unsigned int fs) > +{ > + unsigned long min_audio_clk = fs * 128; > + unsigned long ser_clk = priv->tmds_clock * 1000; > + u8 adiv; > + > + for (adiv = AUDIO_DIV_SERCLK_32; adiv != AUDIO_DIV_SERCLK_1; adiv--) > + if (ser_clk > min_audio_clk << adiv) > + break; > + > + dev_dbg(&priv->hdmi->dev, > + "ser_clk=%luHz fs=%uHz min_aclk=%luHz adiv=%d\n", > + ser_clk, fs, min_audio_clk, adiv); > + > + return adiv; > > Doesn't this function need an error return in case min_audio_clk > ser_clk ? > Or is that a situation that can never arise? It's possible it could arise. For example, if we have a 192kHz sample rate, and a tmds clock slower than 24.576MHz. In such a case, we will select AUDIO_DIV_SERCLK_1 as the divisor, which is a legal value. The result _may_ be audio not working (which is what already happens today when we get this setting wrong.) If we were to return an error, there's no way to handle that error, so what's the point of returning an error? The results of this function match what the Philips firmware uses for this register for all standard TMDS clocks and sample rates, so it's not a problem that I'm particularly concerned about at this point. The only way around this would be to increase the TMDS clock, which means using pixel doubling tricks, and therefore a mode set. I don't think any HDMI driver has the capability to deal with that yet. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel