Re: [PATCH v2] drm/i915: Audio N value computed for pixel doubled modes

[Clint Taylor <clinton.a.taylor@intel.com>]

On 10/07/2014 01:52 AM, Ville Syrjälä wrote:
> On Mon, Oct 06, 2014 at 03:01:46PM -0700, Clint Taylor wrote:
>> On 09/26/2014 09:28 AM, Ville Syrjälä wrote:
>>> On Thu, Sep 25, 2014 at 09:26:36AM -0700, clinton.a.taylor@intel.com wrote:
>>>> From: Clint Taylor <clinton.a.taylor@intel.com>
>>>>
>>>> HDMI audio clock config was incorrectly choosing the default for
>>>> pixel doubled interlaced modes. The table was missing pixel clock
>>>> values 13.500 (27.000) and 13.513 (27.027). Luckily the default N
>>>> value for 25.200 is the same N value for both 27MHz pixel clocks,
>>>> a warning message was being printed with drm.debug set.
>>>>
>>>> ver2: Use 13500 * 1001 / 1000 instead of 13513 constant.
>>>>
>>>> Cc: Jani Nikula <jani.nikula at intel.com>
>>>>
>>>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/intel_display.c |    2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>>> index 858011d..e76a4106 100644
>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>> @@ -7872,6 +7872,8 @@ static struct {
>>>>    	{ 74250, AUD_CONFIG_PIXEL_CLOCK_HDMI_74250 },
>>>>    	{ DIV_ROUND_UP(148500 * 1000, 1001), AUD_CONFIG_PIXEL_CLOCK_HDMI_148352 },
>>>>    	{ 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 },
>>>> +	{ 13500, AUD_CONFIG_PIXEL_CLOCK_HDMI_27000 },
>>>> +	{ 13500 * 1001 / 1000, AUD_CONFIG_PIXEL_CLOCK_HDMI_27027 },
>>>
>>> We have double clocked modes where the non-doubled clock is already
>>> 27MHz so this seems like a bandaid for one particular case rather than a
>>> full solution.
>>>
>>> The HDMI specification makes it clear that the N/CTS stuff depends on
>>> the TMDS clock and not the pixel clock, but BSpec just talks about pixel
>>> clock without further explaining any of this stuff. So should we look
>>> at port_clock rather than the pixel clock here?
>>
>> No, Since we support 12bpc we still need to only worry about the pixel
>> clock. TMDS clock and port clock will be multiplied by 1.5 in 12bpc
>> mode, but the N value remains the same as the 8bpc clock.
>
> Does the hardware then have two different N value tables and it picks
> one based on 8bpc vs. 12bpc? Also I'm not sure what happens with
> different audio sample rates, but maybe we only support 48kHz? Sadly
> the spec is quite lacking when it comes to the audio side.
>
The BSPEC also contains manual n/cts registers for non CEA modes. 
Hopefully the HW automatically selects 12bpc N values. The HW does 
select 8bpc other sample rates, so 12bpc 32 and 44.1 "should" work.

> The recommended N values in the HDMI spec do differ somewhat between
> 8bpc and 12bpc, so either we end up using a non-recommended N value
> in 12bpc, or the hardware does some extra magic.
>
Just saw Appendix D in the HDMI spec..