Re: [PATCH v2 2/2] drm/i915: Add N & CTS values for 10/12 bit deep color

[Clinton Taylor <Clinton.A.Taylor@intel.com>]

On 4/30/19 2:25 AM, Jani Nikula wrote:
> On Thu, 04 Apr 2019, Aditya Swarup <aditya.swarup@intel.com> wrote:
>> On Tue, Apr 02, 2019 at 05:14:40AM -0700, Aditya Swarup wrote:
>>> Adding N & CTS values for 10/12 bit deep color from Appendix C
>>> table in HDMI 2.0 spec. The correct values for N is not chosen
>>> automatically by hardware for deep color modes.
>>>
>>> v2: Remove redundant code and make it generic.(Jani)
>>>
>>> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
>>> Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_audio.c | 82 +++++++++++++++++++++++++-----
>>>   1 file changed, 69 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>>> index 502b57ce72ab..ad53b04fa5a2 100644
>>> --- a/drivers/gpu/drm/i915/intel_audio.c
>>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>>> @@ -70,6 +70,13 @@ struct dp_aud_n_m {
>>>   	u16 n;
>>>   };
>>>   
>>> +struct hdmi_aud_ncts_table {
>>> +	int sample_rate;
>>> +	int clock;
>>> +	int n;
>>> +	int cts;
>>> +};
>>> +
>>>   /* Values according to DP 1.4 Table 2-104 */
>>>   static const struct dp_aud_n_m dp_aud_n_m[] = {
>>>   	{ 32000, LC_162M, 1024, 10125 },
>>> @@ -146,12 +153,7 @@ static const struct {
>>>   #define TMDS_594M 594000
>>>   #define TMDS_593M 593407
>>>   
>>> -static const struct {
>>> -	int sample_rate;
>>> -	int clock;
>>> -	int n;
>>> -	int cts;
>>> -} hdmi_aud_ncts[] = {
>>> +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_24bpp[] = {
>>>   	{ 32000, TMDS_296M, 5824, 421875 },
>>>   	{ 32000, TMDS_297M, 3072, 222750 },
>>>   	{ 32000, TMDS_593M, 5824, 843750 },
>>> @@ -182,6 +184,49 @@ static const struct {
>>>   	{ 192000, TMDS_594M, 24576, 594000 },
>>>   };
>>>   
>>> +/* Appendix C - N & CTS values for deep color from HDMI 2.0 spec*/
>>> +/* HDMI N/CTS table for 10 bit deep color(30 bpp)*/
>>> +#define TMDS_371M 371250
>>> +#define TMDS_370M 370878
>>> +
>>> +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_30bpp[] = {
>>> +	{ 32000, TMDS_370M, 5824, 527344 },
>>> +	{ 32000, TMDS_371M, 6144, 556875 },
>>> +	{ 44100, TMDS_370M, 8918, 585938 },
>>> +	{ 44100, TMDS_371M, 4704, 309375 },
>>> +	{ 88200, TMDS_370M, 17836, 585938 },
>>> +	{ 88200, TMDS_371M, 9408, 309375 },
>>> +	{ 176400, TMDS_370M, 35672, 585938 },
>>> +	{ 176400, TMDS_371M, 18816, 309375 },
>>> +	{ 48000, TMDS_370M, 11648, 703125 },
>>> +	{ 48000, TMDS_371M, 5120, 309375 },
>>> +	{ 96000, TMDS_370M, 23296, 703125 },
>>> +	{ 96000, TMDS_371M, 10240, 309375 },
>>> +	{ 192000, TMDS_370M, 46592, 703125 },
>>> +	{ 192000, TMDS_371M, 20480, 309375 },
>>> +};
>>> +
>>> +/* HDMI N/CTS table for 12 bit deep color(36 bpp)*/
>>> +#define TMDS_445_5M 445500
>>> +#define TMDS_445M 445054
>>> +
>>> +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_36bpp[] = {
>>> +	{ 32000, TMDS_445M, 5824, 632813 },
>>> +	{ 32000, TMDS_445_5M, 4096, 445500 },
>>> +	{ 44100, TMDS_445M, 8918, 703125 },
>>> +	{ 44100, TMDS_445_5M, 4704, 371250 },
>>> +	{ 88200, TMDS_445M, 17836, 703125 },
>>> +	{ 88200, TMDS_445_5M, 9408, 371250 },
>>> +	{ 176400, TMDS_445M, 35672, 703125 },
>>> +	{ 176400, TMDS_445_5M, 18816, 371250 },
>>> +	{ 48000, TMDS_445M, 5824, 421875 },
>>> +	{ 48000, TMDS_445_5M, 5120, 371250 },
>>> +	{ 96000, TMDS_445M, 11648, 421875 },
>>> +	{ 96000, TMDS_445_5M, 10240, 371250 },
>>> +	{ 192000, TMDS_445M, 23296, 421875 },
>>> +	{ 192000, TMDS_445_5M, 20480, 371250 },
>>> +};
>>> +
>>>   /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
>>>   static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state *crtc_state)
>>>   {
>>> @@ -210,16 +255,27 @@ static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state *crtc_sta
>>>   static int audio_config_hdmi_get_n(const struct intel_crtc_state *crtc_state,
>>>   				   int rate)
>>>   {
>>> -	const struct drm_display_mode *adjusted_mode =
>>> -		&crtc_state->base.adjusted_mode;
>>> -	int i;
>>> +	const struct hdmi_aud_ncts_table *hdmi_ncts_table;
>>> +	int i, size = 0;
>>> +
>>> +	if (crtc_state->pipe_bpp == 36) {
>>> +		hdmi_ncts_table = hdmi_aud_ncts_36bpp;
>>> +		size = ARRAY_SIZE(hdmi_aud_ncts_36bpp);
>>> +	} else if (crtc_state->pipe_bpp == 30) {
>>> +		hdmi_ncts_table = hdmi_aud_ncts_30bpp;
>>> +		size = ARRAY_SIZE(hdmi_aud_ncts_30bpp);
>>> +	} else {
>>> +		hdmi_ncts_table = hdmi_aud_ncts_24bpp;
>>> +		size = ARRAY_SIZE(hdmi_aud_ncts_24bpp);
>>> +	}
>>>   
>>> -	for (i = 0; i < ARRAY_SIZE(hdmi_aud_ncts); i++) {
>>> -		if (rate == hdmi_aud_ncts[i].sample_rate &&
>>> -		    adjusted_mode->crtc_clock == hdmi_aud_ncts[i].clock) {
>>> -			return hdmi_aud_ncts[i].n;
>>> +	for (i = 0;  i < size; i++) {
>>> +		if (rate == hdmi_ncts_table[i].sample_rate &&
>>> +		    crtc_state->port_clock == hdmi_ncts_table[i].clock) {
>>> +			return hdmi_ncts_table[i].n;
>>>   		}
>>>   	}
>>> +
>>>   	return 0;
>>>   }
>>>   
>>> -- 
>>> 2.17.1
>>>
>> Jani
>> Do you want me to change anything in this patch?
> Hey, please don't drop things on the floor if you don't hear from me!
>
> I didn't look at the spec, but it seems odd to me that the bpp would
> limit us to specific port clocks. Is that really so? What if you have,
> say, TMDS_594M and 36 bpp, this wouldn't find the params.

TMDS_594M and 36bpp is not possible. 594 x 1.5 (36 bit) would require a 
891MHz TMDS clock and the Max TMDS in the HDMI spec is 594 MHz. .

-Clint


>
> BR,
> Jani.
>
>
>> Regards,
>> Aditya Swarup
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx