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

Clinton Taylor
2019-04-30T15:55:23Z

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