| Message ID | 20190402121440.20116-2-aditya.swarup@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] drm/i915/icl: Set GCP_COLOR_INDICATION only for 10/12 bit deep color | expand |
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? Regards, Aditya Swarup
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. BR, Jani. > > Regards, > Aditya Swarup
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
On Tue, 02 Apr 2019, Aditya Swarup <aditya.swarup@intel.com> 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 { The struct itself is not a table. Just make it struct hdmi_ncts or something. > + 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 }, N should be 11648? > + { 32000, TMDS_371M, 6144, 556875 }, > + { 44100, TMDS_370M, 8918, 585938 }, N 17836 > + { 44100, TMDS_371M, 4704, 309375 }, > + { 88200, TMDS_370M, 17836, 585938 }, N 35672 > + { 88200, TMDS_371M, 9408, 309375 }, > + { 176400, TMDS_370M, 35672, 585938 }, N 71344 > + { 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; Unnecessary initialization. > + > + 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++) { ^ Superfluous space. > + if (rate == hdmi_ncts_table[i].sample_rate && > + crtc_state->port_clock == hdmi_ncts_table[i].clock) { Changing from adjusted_mode->crtc_clock to crtc_state->port_clock is a separate change that needs to be a separate patch. BR, Jani. > + return hdmi_ncts_table[i].n; > } > } > + > return 0; > }
On Wed, May 15, 2019 at 05:40:10PM +0300, Jani Nikula wrote: > On Tue, 02 Apr 2019, Aditya Swarup <aditya.swarup@intel.com> 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 { > > The struct itself is not a table. Just make it struct hdmi_ncts or > something. Will do it in the next spin. > > > + 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 }, > > N should be 11648? The value in the struct is correct. It is 5824 - Appendix C Table C-1 Pg 234, for 371.25/1.001 and f = 32kHz => N = 5824 > > > + { 32000, TMDS_371M, 6144, 556875 }, > > + { 44100, TMDS_370M, 8918, 585938 }, > > N 17836 Value in the table is correct. > > > + { 44100, TMDS_371M, 4704, 309375 }, > > + { 88200, TMDS_370M, 17836, 585938 }, > > N 35672 Value in the table is correct. > > > + { 88200, TMDS_371M, 9408, 309375 }, > > + { 176400, TMDS_370M, 35672, 585938 }, > > N 71344 Value in the table is correct. > > > + { 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; > > Unnecessary initialization. > > > + > > + 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++) { > ^ > Superfluous space. Will correct it in next spin. > > > > + if (rate == hdmi_ncts_table[i].sample_rate && > > + crtc_state->port_clock == hdmi_ncts_table[i].clock) { > > Changing from adjusted_mode->crtc_clock to crtc_state->port_clock is a > separate change that needs to be a separate patch. I don't think it should be a separate change. Working with deep color mode requires crtc_state->port_clock as it is scaled according to the deep color mode set. Using adjusted_mode->crtc_clock is incorrect as it doesn't account for deep color mode; we will have to multiply with scaling factor for the set deep color mode which is superfluous. I can add a comment in the commit message and comment before the if block to explain the change. Let me know if it works? > > > BR, > Jani. > > > + return hdmi_ncts_table[i].n; > > } > > } > > + > > return 0; > > } > > -- > Jani Nikula, Intel Open Source Graphics Center Regards, Aditya Swarup
On Fri, 17 May 2019, Aditya Swarup <aditya.swarup@intel.com> wrote: > On Wed, May 15, 2019 at 05:40:10PM +0300, Jani Nikula wrote: >> On Tue, 02 Apr 2019, Aditya Swarup <aditya.swarup@intel.com> 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 { >> >> The struct itself is not a table. Just make it struct hdmi_ncts or >> something. > Will do it in the next spin. >> >> > + 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 }, >> >> N should be 11648? > The value in the struct is correct. It is 5824 - Appendix C Table C-1 Pg > 234, for 371.25/1.001 and f = 32kHz => N = 5824 Huh, looks like I have an outdated version of the spec then. For me the table is on page 232. Need to find the up-to-date spec, but looking at draft 2.1 the values you have match. >> >> > + { 32000, TMDS_371M, 6144, 556875 }, >> > + { 44100, TMDS_370M, 8918, 585938 }, >> >> N 17836 > Value in the table is correct. >> >> > + { 44100, TMDS_371M, 4704, 309375 }, >> > + { 88200, TMDS_370M, 17836, 585938 }, >> >> N 35672 > Value in the table is correct. >> >> > + { 88200, TMDS_371M, 9408, 309375 }, >> > + { 176400, TMDS_370M, 35672, 585938 }, >> >> N 71344 > Value in the table is correct. >> >> > + { 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; >> >> Unnecessary initialization. >> >> > + >> > + 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++) { >> ^ >> Superfluous space. > Will correct it in next spin. >> >> >> > + if (rate == hdmi_ncts_table[i].sample_rate && >> > + crtc_state->port_clock == hdmi_ncts_table[i].clock) { >> >> Changing from adjusted_mode->crtc_clock to crtc_state->port_clock is a >> separate change that needs to be a separate patch. > I don't think it should be a separate change. Working with deep color > mode requires crtc_state->port_clock as it is scaled according to the > deep color mode set. Using adjusted_mode->crtc_clock is incorrect as it > doesn't account for deep color mode; we will have to multiply with > scaling factor for the set deep color mode which is superfluous. Make the crtc_clock -> port_clock change *before* the deep color changes, that's the whole point. Ensure that this change does not impact non-deep color, and if it does, the bisect will point at the minimal change. Then add the deep color N & CTS when the code already uses port_clock. BR, Jani. > > I can add a comment in the commit message and comment before the if > block to explain the change. Let me know if it works? >> >> >> BR, >> Jani. >> >> > + return hdmi_ncts_table[i].n; >> > } >> > } >> > + >> > return 0; >> > } >> >> -- >> Jani Nikula, Intel Open Source Graphics Center > > Regards, > Aditya Swarup
On Fri, 17 May 2019, Jani Nikula <jani.nikula@intel.com> wrote: > On Fri, 17 May 2019, Aditya Swarup <aditya.swarup@intel.com> wrote: >> On Wed, May 15, 2019 at 05:40:10PM +0300, Jani Nikula wrote: >>> On Tue, 02 Apr 2019, Aditya Swarup <aditya.swarup@intel.com> 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 { >>> >>> The struct itself is not a table. Just make it struct hdmi_ncts or >>> something. >> Will do it in the next spin. Aditya, please provide the updated version. BR, Jani. >>> >>> > + 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 }, >>> >>> N should be 11648? >> The value in the struct is correct. It is 5824 - Appendix C Table C-1 Pg >> 234, for 371.25/1.001 and f = 32kHz => N = 5824 > > Huh, looks like I have an outdated version of the spec then. For me the > table is on page 232. > > Need to find the up-to-date spec, but looking at draft 2.1 the values > you have match. > >>> >>> > + { 32000, TMDS_371M, 6144, 556875 }, >>> > + { 44100, TMDS_370M, 8918, 585938 }, >>> >>> N 17836 >> Value in the table is correct. >>> >>> > + { 44100, TMDS_371M, 4704, 309375 }, >>> > + { 88200, TMDS_370M, 17836, 585938 }, >>> >>> N 35672 >> Value in the table is correct. >>> >>> > + { 88200, TMDS_371M, 9408, 309375 }, >>> > + { 176400, TMDS_370M, 35672, 585938 }, >>> >>> N 71344 >> Value in the table is correct. >>> >>> > + { 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; >>> >>> Unnecessary initialization. >>> >>> > + >>> > + 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++) { >>> ^ >>> Superfluous space. >> Will correct it in next spin. >>> >>> >>> > + if (rate == hdmi_ncts_table[i].sample_rate && >>> > + crtc_state->port_clock == hdmi_ncts_table[i].clock) { >>> >>> Changing from adjusted_mode->crtc_clock to crtc_state->port_clock is a >>> separate change that needs to be a separate patch. >> I don't think it should be a separate change. Working with deep color >> mode requires crtc_state->port_clock as it is scaled according to the >> deep color mode set. Using adjusted_mode->crtc_clock is incorrect as it >> doesn't account for deep color mode; we will have to multiply with >> scaling factor for the set deep color mode which is superfluous. > > Make the crtc_clock -> port_clock change *before* the deep color > changes, that's the whole point. Ensure that this change does not impact > non-deep color, and if it does, the bisect will point at the minimal > change. Then add the deep color N & CTS when the code already uses > port_clock. > > BR, > Jani. > >> >> I can add a comment in the commit message and comment before the if >> block to explain the change. Let me know if it works? >>> >>> >>> BR, >>> Jani. >>> >>> > + return hdmi_ncts_table[i].n; >>> > } >>> > } >>> > + >>> > return 0; >>> > } >>> >>> -- >>> Jani Nikula, Intel Open Source Graphics Center >> >> Regards, >> Aditya Swarup
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; }
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(-)