diff mbox

drm/i915: 4K audio N value incorrect at 29.97 and 23.98 refresh rate

Message ID 1444253909-22562-1-git-send-email-clinton.a.taylor@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Taylor, Clinton A Oct. 7, 2015, 9:38 p.m. UTC
From: Clint Taylor <clinton.a.taylor@intel.com>

The TMDS_296M define was computing as 296704 but the mode->clock is
296700 as defined by EDID. Adjusted define to allow correct detection
of the need to program the correct N value for 29.97 and 23.98 refresh
rate.

Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
---
 drivers/gpu/drm/i915/intel_audio.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ville Syrjälä Oct. 8, 2015, 7:27 a.m. UTC | #1
On Wed, Oct 07, 2015 at 02:38:29PM -0700, clinton.a.taylor@intel.com wrote:
> From: Clint Taylor <clinton.a.taylor@intel.com>
> 
> The TMDS_296M define was computing as 296704 but the mode->clock is
> 296700 as defined by EDID. Adjusted define to allow correct detection
> of the need to program the correct N value for 29.97 and 23.98 refresh
> rate.
> 
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 56c2f54..419c979 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -75,7 +75,7 @@ static const struct {
>  
>  /* HDMI N/CTS table */
>  #define TMDS_297M 297000
> -#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001)
> +#define TMDS_296M 296700

Hmm. I think we might want to fix up the mode instead. Let me post a few
patches, and let's see what people think...

>  static const struct {
>  	int sample_rate;
>  	int clock;
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Jani Nikula Oct. 8, 2015, 10:59 a.m. UTC | #2
On Thu, 08 Oct 2015, clinton.a.taylor@intel.com wrote:
> From: Clint Taylor <clinton.a.taylor@intel.com>
>
> The TMDS_296M define was computing as 296704 but the mode->clock is
> 296700 as defined by EDID. Adjusted define to allow correct detection
> of the need to program the correct N value for 29.97 and 23.98 refresh
> rate.

<insert bad commit id here>


>
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 56c2f54..419c979 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -75,7 +75,7 @@ static const struct {
>  
>  /* HDMI N/CTS table */
>  #define TMDS_297M 297000
> -#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001)
> +#define TMDS_296M 296700
>  static const struct {
>  	int sample_rate;
>  	int clock;
> -- 
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Jani Nikula Oct. 8, 2015, 12:39 p.m. UTC | #3
On Thu, 08 Oct 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Oct 07, 2015 at 02:38:29PM -0700, clinton.a.taylor@intel.com wrote:
>> From: Clint Taylor <clinton.a.taylor@intel.com>
>> 
>> The TMDS_296M define was computing as 296704 but the mode->clock is
>> 296700 as defined by EDID. Adjusted define to allow correct detection
>> of the need to program the correct N value for 29.97 and 23.98 refresh
>> rate.
>> 
>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_audio.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>> index 56c2f54..419c979 100644
>> --- a/drivers/gpu/drm/i915/intel_audio.c
>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>> @@ -75,7 +75,7 @@ static const struct {
>>  
>>  /* HDMI N/CTS table */
>>  #define TMDS_297M 297000
>> -#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001)
>> +#define TMDS_296M 296700
>
> Hmm. I think we might want to fix up the mode instead. Let me post a few
> patches, and let's see what people think...

We might actually want to queue this up for v4.3 as the immediate fix,
as I feel it's doubtful your other patches will make it to v4.3.

If Someone(tm) provided their r-b that is.

BR,
Jani.


>
>>  static const struct {
>>  	int sample_rate;
>>  	int clock;
>> -- 
>> 1.7.9.5
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjälä Oct. 8, 2015, 12:44 p.m. UTC | #4
On Thu, Oct 08, 2015 at 03:39:26PM +0300, Jani Nikula wrote:
> On Thu, 08 Oct 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Wed, Oct 07, 2015 at 02:38:29PM -0700, clinton.a.taylor@intel.com wrote:
> >> From: Clint Taylor <clinton.a.taylor@intel.com>
> >> 
> >> The TMDS_296M define was computing as 296704 but the mode->clock is
> >> 296700 as defined by EDID. Adjusted define to allow correct detection
> >> of the need to program the correct N value for 29.97 and 23.98 refresh
> >> rate.
> >> 
> >> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_audio.c |    2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> >> index 56c2f54..419c979 100644
> >> --- a/drivers/gpu/drm/i915/intel_audio.c
> >> +++ b/drivers/gpu/drm/i915/intel_audio.c
> >> @@ -75,7 +75,7 @@ static const struct {
> >>  
> >>  /* HDMI N/CTS table */
> >>  #define TMDS_297M 297000
> >> -#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001)
> >> +#define TMDS_296M 296700
> >
> > Hmm. I think we might want to fix up the mode instead. Let me post a few
> > patches, and let's see what people think...
> 
> We might actually want to queue this up for v4.3 as the immediate fix,
> as I feel it's doubtful your other patches will make it to v4.3.
> 
> If Someone(tm) provided their r-b that is.

Except this is wrong if the mode came from the CEA/HDMI mode list as
opposed from the EDID detailed timings.
Taylor, Clinton A Oct. 8, 2015, 4:06 p.m. UTC | #5
On 10/08/2015 05:44 AM, Ville Syrjälä wrote:
> On Thu, Oct 08, 2015 at 03:39:26PM +0300, Jani Nikula wrote:
>> On Thu, 08 Oct 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>>> On Wed, Oct 07, 2015 at 02:38:29PM -0700, clinton.a.taylor@intel.com wrote:
>>>> From: Clint Taylor <clinton.a.taylor@intel.com>
>>>>
>>>> The TMDS_296M define was computing as 296704 but the mode->clock is
>>>> 296700 as defined by EDID. Adjusted define to allow correct detection
>>>> of the need to program the correct N value for 29.97 and 23.98 refresh
>>>> rate.
>>>>
>>>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>>>> ---
>>>>   drivers/gpu/drm/i915/intel_audio.c |    2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>>>> index 56c2f54..419c979 100644
>>>> --- a/drivers/gpu/drm/i915/intel_audio.c
>>>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>>>> @@ -75,7 +75,7 @@ static const struct {
>>>>
>>>>   /* HDMI N/CTS table */
>>>>   #define TMDS_297M 297000
>>>> -#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001)
>>>> +#define TMDS_296M 296700
>>>
>>> Hmm. I think we might want to fix up the mode instead. Let me post a few
>>> patches, and let's see what people think...
>>
>> We might actually want to queue this up for v4.3 as the immediate fix,
>> as I feel it's doubtful your other patches will make it to v4.3.
>>
>> If Someone(tm) provided their r-b that is.
>
> Except this is wrong if the mode came from the CEA/HDMI mode list as
> opposed from the EDID detailed timings.
>
The original patch was generated from a monitor with 296700 in the DTD. 
Ville's approach is more sane for the long term. We could also just add 
a third check in the 4K audio fixup for the DTD based pixel clock.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 56c2f54..419c979 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -75,7 +75,7 @@  static const struct {
 
 /* HDMI N/CTS table */
 #define TMDS_297M 297000
-#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001)
+#define TMDS_296M 296700
 static const struct {
 	int sample_rate;
 	int clock;