| Message ID | 20171114194759.24541-5-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Nov 14, 2017 at 11:47:56AM -0800, Rodrigo Vivi wrote: > Spec describe all values in MHz. We handle our > clocks in KHz. This includes the best_dco_centrality that was > forgot in the same unity as spec. Consequently we couldn't > get a good divider for high frequenies. Hence HDMI 2.0 wasn't > working. > > This patch also replaces the use of "* KHz(1)" with the values > directly on KHz to avoid future confusion. > > Cc: Shashank Sharma <shashank.sharma@intel.com> > Cc: Mika Kahola <mika.kahola@intel.com> > Cc: Manasi Navare <manasi.d.navare@intel.com> > Cc: James Ausmus <james.ausmus@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/intel_dpll_mgr.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c > index fba969cbda37..53f650f56148 100644 > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > @@ -2201,8 +2201,8 @@ cnl_ddi_calculate_wrpll(int clock, > struct skl_wrpll_params *wrpll_params) > { > u32 afe_clock = clock * 5; > - u32 dco_min = 7998 * KHz(1); > - u32 dco_max = 10000 * KHz(1); > + u32 dco_min = 7998000; > + u32 dco_max = 10000000; > u32 dco_mid = (dco_min + dco_max) / 2; > static const int dividers[] = { 2, 4, 6, 8, 10, 12, 14, 16, > 18, 20, 24, 28, 30, 32, 36, 40, > @@ -2211,7 +2211,7 @@ cnl_ddi_calculate_wrpll(int clock, > 84, 88, 90, 92, 96, 98, 100, 102, > 3, 5, 7, 9, 15, 21 }; > u32 dco, best_dco = 0, dco_centrality = 0; > - u32 best_dco_centrality = 999999; > + u32 best_dco_centrality = 999999000; UINT_MAX, -1, or ~0 maybe? > int d, best_div = 0, pdiv = 0, qdiv = 0, kdiv = 0; > > for (d = 0; d < ARRAY_SIZE(dividers); d++) { > -- > 2.13.6 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Nov 14, 2017 at 08:00:31PM +0000, Ville Syrjälä wrote: > On Tue, Nov 14, 2017 at 11:47:56AM -0800, Rodrigo Vivi wrote: > > Spec describe all values in MHz. We handle our > > clocks in KHz. This includes the best_dco_centrality that was > > forgot in the same unity as spec. Consequently we couldn't > > get a good divider for high frequenies. Hence HDMI 2.0 wasn't > > working. > > > > This patch also replaces the use of "* KHz(1)" with the values > > directly on KHz to avoid future confusion. > > > > Cc: Shashank Sharma <shashank.sharma@intel.com> > > Cc: Mika Kahola <mika.kahola@intel.com> > > Cc: Manasi Navare <manasi.d.navare@intel.com> > > Cc: James Ausmus <james.ausmus@intel.com> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > --- > > drivers/gpu/drm/i915/intel_dpll_mgr.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c > > index fba969cbda37..53f650f56148 100644 > > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > > @@ -2201,8 +2201,8 @@ cnl_ddi_calculate_wrpll(int clock, > > struct skl_wrpll_params *wrpll_params) > > { > > u32 afe_clock = clock * 5; > > - u32 dco_min = 7998 * KHz(1); > > - u32 dco_max = 10000 * KHz(1); > > + u32 dco_min = 7998000; > > + u32 dco_max = 10000000; > > u32 dco_mid = (dco_min + dco_max) / 2; > > static const int dividers[] = { 2, 4, 6, 8, 10, 12, 14, 16, > > 18, 20, 24, 28, 30, 32, 36, 40, > > @@ -2211,7 +2211,7 @@ cnl_ddi_calculate_wrpll(int clock, > > 84, 88, 90, 92, 96, 98, 100, 102, > > 3, 5, 7, 9, 15, 21 }; > > u32 dco, best_dco = 0, dco_centrality = 0; > > - u32 best_dco_centrality = 999999; > > + u32 best_dco_centrality = 999999000; > > UINT_MAX, -1, or ~0 maybe? yeap, I considered the max macros, but I didn't want to deviate from spec... > > > int d, best_div = 0, pdiv = 0, qdiv = 0, kdiv = 0; > > > > for (d = 0; d < ARRAY_SIZE(dividers); d++) { > > -- > > 2.13.6 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC
On Tue, Nov 14, 2017 at 12:09:39PM -0800, Rodrigo Vivi wrote: > On Tue, Nov 14, 2017 at 08:00:31PM +0000, Ville Syrjälä wrote: > > On Tue, Nov 14, 2017 at 11:47:56AM -0800, Rodrigo Vivi wrote: > > > Spec describe all values in MHz. We handle our > > > clocks in KHz. This includes the best_dco_centrality that was > > > forgot in the same unity as spec. Consequently we couldn't > > > get a good divider for high frequenies. Hence HDMI 2.0 wasn't > > > working. > > > > > > This patch also replaces the use of "* KHz(1)" with the values > > > directly on KHz to avoid future confusion. > > > > > > Cc: Shashank Sharma <shashank.sharma@intel.com> > > > Cc: Mika Kahola <mika.kahola@intel.com> > > > Cc: Manasi Navare <manasi.d.navare@intel.com> > > > Cc: James Ausmus <james.ausmus@intel.com> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_dpll_mgr.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c > > > index fba969cbda37..53f650f56148 100644 > > > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > > > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > > > @@ -2201,8 +2201,8 @@ cnl_ddi_calculate_wrpll(int clock, > > > struct skl_wrpll_params *wrpll_params) > > > { > > > u32 afe_clock = clock * 5; > > > - u32 dco_min = 7998 * KHz(1); > > > - u32 dco_max = 10000 * KHz(1); > > > + u32 dco_min = 7998000; > > > + u32 dco_max = 10000000; > > > u32 dco_mid = (dco_min + dco_max) / 2; > > > static const int dividers[] = { 2, 4, 6, 8, 10, 12, 14, 16, > > > 18, 20, 24, 28, 30, 32, 36, 40, > > > @@ -2211,7 +2211,7 @@ cnl_ddi_calculate_wrpll(int clock, > > > 84, 88, 90, 92, 96, 98, 100, 102, > > > 3, 5, 7, 9, 15, 21 }; > > > u32 dco, best_dco = 0, dco_centrality = 0; > > > - u32 best_dco_centrality = 999999; > > > + u32 best_dco_centrality = 999999000; > > > > UINT_MAX, -1, or ~0 maybe? > > yeap, I considered the max macros, but I didn't want to deviate > from spec... Always bothers me to see some kind of 999... decimal max value pulled out from someone's hat when they obvious thing clearly is just "max representable value". Feels like someone wasn't thinking 100% when they wrote the spec :) Maybe just extend the 9's all the way to the end then? Dunno. I think I'll just move on from the DDI DPLL code (that apporach has worked pretty well thus far ;) > > > > > > int d, best_div = 0, pdiv = 0, qdiv = 0, kdiv = 0; > > > > > > for (d = 0; d < ARRAY_SIZE(dividers); d++) { > > > -- > > > 2.13.6 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Ville Syrjälä > > Intel OTC
On Tue, 2017-11-14 at 11:47 -0800, Rodrigo Vivi wrote: > Spec describe all values in MHz. We handle our > clocks in KHz. This includes the best_dco_centrality that was > forgot in the same unity as spec. Consequently we couldn't > get a good divider for high frequenies. Hence HDMI 2.0 wasn't > working. > > This patch also replaces the use of "* KHz(1)" with the values > directly on KHz to avoid future confusion. > > Cc: Shashank Sharma <shashank.sharma@intel.com> > Cc: Mika Kahola <mika.kahola@intel.com> > Cc: Manasi Navare <manasi.d.navare@intel.com> > Cc: James Ausmus <james.ausmus@intel.com> Reviewed-by: Mika Kahola <mika.kahola@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/intel_dpll_mgr.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c > b/drivers/gpu/drm/i915/intel_dpll_mgr.c > index fba969cbda37..53f650f56148 100644 > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > @@ -2201,8 +2201,8 @@ cnl_ddi_calculate_wrpll(int clock, > struct skl_wrpll_params *wrpll_params) > { > u32 afe_clock = clock * 5; > - u32 dco_min = 7998 * KHz(1); > - u32 dco_max = 10000 * KHz(1); > + u32 dco_min = 7998000; > + u32 dco_max = 10000000; > u32 dco_mid = (dco_min + dco_max) / 2; > static const int dividers[] = { 2, 4, 6, 8, 10, > 12, 14, 16, > 18, 20, 24, 28, 30, > 32, 36, 40, > @@ -2211,7 +2211,7 @@ cnl_ddi_calculate_wrpll(int clock, > 84, 88, 90, 92, 96, 98, > 100, 102, > 3, 5, 7, 9, 15, 21 }; > u32 dco, best_dco = 0, dco_centrality = 0; > - u32 best_dco_centrality = 999999; > + u32 best_dco_centrality = 999999000; > int d, best_div = 0, pdiv = 0, qdiv = 0, kdiv = 0; > > for (d = 0; d < ARRAY_SIZE(dividers); d++) {
On Wed, Nov 15, 2017 at 08:06:16AM +0000, Mika Kahola wrote: > On Tue, 2017-11-14 at 11:47 -0800, Rodrigo Vivi wrote: > > Spec describe all values in MHz. We handle our > > clocks in KHz. This includes the best_dco_centrality that was > > forgot in the same unity as spec. Consequently we couldn't > > get a good divider for high frequenies. Hence HDMI 2.0 wasn't > > working. > > > > This patch also replaces the use of "* KHz(1)" with the values > > directly on KHz to avoid future confusion. > > > > Cc: Shashank Sharma <shashank.sharma@intel.com> > > Cc: Mika Kahola <mika.kahola@intel.com> > > Cc: Manasi Navare <manasi.d.navare@intel.com> > > Cc: James Ausmus <james.ausmus@intel.com> > Reviewed-by: Mika Kahola <mika.kahola@intel.com> 999999000 or U32_MAX? > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > --- > > drivers/gpu/drm/i915/intel_dpll_mgr.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c > > index fba969cbda37..53f650f56148 100644 > > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > > @@ -2201,8 +2201,8 @@ cnl_ddi_calculate_wrpll(int clock, > > struct skl_wrpll_params *wrpll_params) > > { > > u32 afe_clock = clock * 5; > > - u32 dco_min = 7998 * KHz(1); > > - u32 dco_max = 10000 * KHz(1); > > + u32 dco_min = 7998000; > > + u32 dco_max = 10000000; > > u32 dco_mid = (dco_min + dco_max) / 2; > > static const int dividers[] = { 2, 4, 6, 8, 10, > > 12, 14, 16, > > 18, 20, 24, 28, 30, > > 32, 36, 40, > > @@ -2211,7 +2211,7 @@ cnl_ddi_calculate_wrpll(int clock, > > 84, 88, 90, 92, 96, 98, > > 100, 102, > > 3, 5, 7, 9, 15, 21 }; > > u32 dco, best_dco = 0, dco_centrality = 0; > > - u32 best_dco_centrality = 999999; > > + u32 best_dco_centrality = 999999000; > > int d, best_div = 0, pdiv = 0, qdiv = 0, kdiv = 0; > > > > for (d = 0; d < ARRAY_SIZE(dividers); d++) { > -- > Mika Kahola - Intel OTC >
On Wed, 2017-11-15 at 10:04 -0800, Rodrigo Vivi wrote: > On Wed, Nov 15, 2017 at 08:06:16AM +0000, Mika Kahola wrote: > > > > On Tue, 2017-11-14 at 11:47 -0800, Rodrigo Vivi wrote: > > > > > > Spec describe all values in MHz. We handle our > > > clocks in KHz. This includes the best_dco_centrality that was > > > forgot in the same unity as spec. Consequently we couldn't > > > get a good divider for high frequenies. Hence HDMI 2.0 wasn't > > > working. > > > > > > This patch also replaces the use of "* KHz(1)" with the values > > > directly on KHz to avoid future confusion. > > > > > > Cc: Shashank Sharma <shashank.sharma@intel.com> > > > Cc: Mika Kahola <mika.kahola@intel.com> > > > Cc: Manasi Navare <manasi.d.navare@intel.com> > > > Cc: James Ausmus <james.ausmus@intel.com> > > Reviewed-by: Mika Kahola <mika.kahola@intel.com> > 999999000 or U32_MAX? The BSpec seems to favor 999999000 so I would follow the spec here. > > > > > > > > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_dpll_mgr.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c > > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c > > > index fba969cbda37..53f650f56148 100644 > > > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > > > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > > > @@ -2201,8 +2201,8 @@ cnl_ddi_calculate_wrpll(int clock, > > > struct skl_wrpll_params *wrpll_params) > > > { > > > u32 afe_clock = clock * 5; > > > - u32 dco_min = 7998 * KHz(1); > > > - u32 dco_max = 10000 * KHz(1); > > > + u32 dco_min = 7998000; > > > + u32 dco_max = 10000000; > > > u32 dco_mid = (dco_min + dco_max) / 2; > > > static const int dividers[] = { 2, 4, 6, 8, 10, > > > 12, 14, 16, > > > 18, 20, 24, 28, 30, > > > 32, 36, 40, > > > @@ -2211,7 +2211,7 @@ cnl_ddi_calculate_wrpll(int clock, > > > 84, 88, 90, 92, 96, 98, > > > 100, 102, > > > 3, 5, 7, 9, 15, 21 > > > }; > > > u32 dco, best_dco = 0, dco_centrality = 0; > > > - u32 best_dco_centrality = 999999; > > > + u32 best_dco_centrality = 999999000; > > > int d, best_div = 0, pdiv = 0, qdiv = 0, kdiv = 0; > > > > > > for (d = 0; d < ARRAY_SIZE(dividers); d++) {
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c index fba969cbda37..53f650f56148 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -2201,8 +2201,8 @@ cnl_ddi_calculate_wrpll(int clock, struct skl_wrpll_params *wrpll_params) { u32 afe_clock = clock * 5; - u32 dco_min = 7998 * KHz(1); - u32 dco_max = 10000 * KHz(1); + u32 dco_min = 7998000; + u32 dco_max = 10000000; u32 dco_mid = (dco_min + dco_max) / 2; static const int dividers[] = { 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 24, 28, 30, 32, 36, 40, @@ -2211,7 +2211,7 @@ cnl_ddi_calculate_wrpll(int clock, 84, 88, 90, 92, 96, 98, 100, 102, 3, 5, 7, 9, 15, 21 }; u32 dco, best_dco = 0, dco_centrality = 0; - u32 best_dco_centrality = 999999; + u32 best_dco_centrality = 999999000; int d, best_div = 0, pdiv = 0, qdiv = 0, kdiv = 0; for (d = 0; d < ARRAY_SIZE(dividers); d++) {
Spec describe all values in MHz. We handle our clocks in KHz. This includes the best_dco_centrality that was forgot in the same unity as spec. Consequently we couldn't get a good divider for high frequenies. Hence HDMI 2.0 wasn't working. This patch also replaces the use of "* KHz(1)" with the values directly on KHz to avoid future confusion. Cc: Shashank Sharma <shashank.sharma@intel.com> Cc: Mika Kahola <mika.kahola@intel.com> Cc: Manasi Navare <manasi.d.navare@intel.com> Cc: James Ausmus <james.ausmus@intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/intel_dpll_mgr.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)