* [PATCH 1/2] drm/i915/cnl: Allow 2 pixel per clock on Cannonlake. @ 2017-08-17 0:54 Rodrigo Vivi 2017-08-17 0:54 ` [PATCH 2/2] drm/i915: Introduce HAS_2PPC Rodrigo Vivi ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Rodrigo Vivi @ 2017-08-17 0:54 UTC (permalink / raw) To: intel-gfx; +Cc: Jani Nikula, Dhinakaran Pandiyan, Rodrigo Vivi This is heavily based on a initial patch provided by Ville plus all changes provided later by Ander. As Geminilake, Cannonlake also supports 2 pixels per clock. Different from Geminilake we are not implementing the 99% Wa. But we can revisit that decision later if we find out any limitation on later CNL SKUs. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/intel_cdclk.c | 12 ++++++------ drivers/gpu/drm/i915/intel_pm.c | 3 ++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c index 1241e5891b29..6b1d805fb755 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -1422,9 +1422,9 @@ void bxt_uninit_cdclk(struct drm_i915_private *dev_priv) static int cnl_calc_cdclk(int max_pixclk) { - if (max_pixclk > 336000) + if (max_pixclk > 2 * 336000) return 528000; - else if (max_pixclk > 168000) + else if (max_pixclk > 2 * 168000) return 336000; else return 168000; @@ -1752,9 +1752,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state, crtc_state->has_audio && crtc_state->port_clock >= 540000 && crtc_state->lane_count == 4) { - if (IS_CANNONLAKE(dev_priv)) - pixel_rate = max(316800, pixel_rate); - else if (IS_GEMINILAKE(dev_priv)) + if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) pixel_rate = max(2 * 316800, pixel_rate); else pixel_rate = max(432000, pixel_rate); @@ -1766,7 +1764,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state, * two pixels per clock. */ if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) { - if (IS_GEMINILAKE(dev_priv)) + if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) pixel_rate = max(2 * 2 * 96000, pixel_rate); else pixel_rate = max(2 * 96000, pixel_rate); @@ -1999,6 +1997,8 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv) { int max_cdclk_freq = dev_priv->max_cdclk_freq; + if (IS_CANNONLAKE(dev_priv)) + return 2 * max_cdclk_freq; if (IS_GEMINILAKE(dev_priv)) /* * FIXME: Limiting to 99% as a temporary workaround. See diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index ed662937ec3c..42f753df30cb 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3969,7 +3969,8 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc, crtc_clock = crtc_state->adjusted_mode.crtc_clock; dotclk = to_intel_atomic_state(state)->cdclk.logical.cdclk; - if (IS_GEMINILAKE(to_i915(intel_crtc->base.dev))) + if (IS_GEMINILAKE(to_i915(intel_crtc->base.dev)) || + IS_CANNONLAKE(to_i915(intel_crtc->base.dev))) dotclk *= 2; pipe_max_pixel_rate = div_round_up_u32_fixed16(dotclk, pipe_downscale); -- 2.13.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] drm/i915: Introduce HAS_2PPC. 2017-08-17 0:54 [PATCH 1/2] drm/i915/cnl: Allow 2 pixel per clock on Cannonlake Rodrigo Vivi @ 2017-08-17 0:54 ` Rodrigo Vivi 2017-08-30 19:12 ` Pandiyan, Dhinakaran 2017-08-17 1:28 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/cnl: Allow 2 pixel per clock on Cannonlake Patchwork 2017-08-30 18:14 ` [PATCH 1/2] " Pandiyan, Dhinakaran 2 siblings, 1 reply; 10+ messages in thread From: Rodrigo Vivi @ 2017-08-17 0:54 UTC (permalink / raw) To: intel-gfx; +Cc: Dhinakaran Pandiyan, Paulo Zanoni, Rodrigo Vivi Let's make it easier to add platforms that supports 2 pixel per clock. With spread checks per platform it was easy to miss one or another spot leading to loose some time on debug. Hopefully this check would save some cases in the future. No functional change. Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 4 ++++ drivers/gpu/drm/i915/i915_pci.c | 2 ++ drivers/gpu/drm/i915/intel_cdclk.c | 8 ++++---- drivers/gpu/drm/i915/intel_pm.c | 3 +-- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 6c25c8520c87..94f5e6522e5e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -748,6 +748,7 @@ struct intel_csr { func(is_lp); \ func(is_alpha_support); \ /* Keep has_* in alphabetical order */ \ + func(has_2ppc); \ func(has_64bit_reloc); \ func(has_aliasing_ppgtt); \ func(has_csr); \ @@ -3025,6 +3026,9 @@ intel_info(const struct drm_i915_private *dev_priv) #define NEEDS_WaRsDisableCoarsePowerGating(dev_priv) \ (IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv)) +/* Supports 2 pixel per clock */ +#define HAS_2PPC(dev_priv) ((dev_priv)->info.has_2ppc) + /* * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts * even when in MSI mode. This results in spurious interrupt warnings if the diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 09d97e0990b7..df84025579cf 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -405,6 +405,7 @@ static const struct intel_device_info intel_geminilake_info = { GEN9_LP_FEATURES, .platform = INTEL_GEMINILAKE, .ddb_size = 1024, + .has_2ppc = 1, .color = { .degamma_lut_size = 0, .gamma_lut_size = 1024 } }; @@ -450,6 +451,7 @@ static const struct intel_device_info intel_cannonlake_info = { .gen = 10, .ddb_size = 1024, .has_csr = 1, + .has_2ppc = 1, .color = { .degamma_lut_size = 0, .gamma_lut_size = 1024 } }; diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c index 6b1d805fb755..edbccda40f0c 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -1752,7 +1752,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state, crtc_state->has_audio && crtc_state->port_clock >= 540000 && crtc_state->lane_count == 4) { - if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) + if (HAS_2PPC(dev_priv)) pixel_rate = max(2 * 316800, pixel_rate); else pixel_rate = max(432000, pixel_rate); @@ -1764,7 +1764,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state, * two pixels per clock. */ if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) { - if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) + if (HAS_2PPC(dev_priv)) pixel_rate = max(2 * 2 * 96000, pixel_rate); else pixel_rate = max(2 * 96000, pixel_rate); @@ -1997,14 +1997,14 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv) { int max_cdclk_freq = dev_priv->max_cdclk_freq; - if (IS_CANNONLAKE(dev_priv)) - return 2 * max_cdclk_freq; if (IS_GEMINILAKE(dev_priv)) /* * FIXME: Limiting to 99% as a temporary workaround. See * glk_calc_cdclk() for details. */ return 2 * max_cdclk_freq * 99 / 100; + else if (HAS_2PPC(dev_priv)) + return 2 * max_cdclk_freq; else if (INTEL_INFO(dev_priv)->gen >= 9 || IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) return max_cdclk_freq; diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 42f753df30cb..c8da6ca4e8df 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3969,8 +3969,7 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc, crtc_clock = crtc_state->adjusted_mode.crtc_clock; dotclk = to_intel_atomic_state(state)->cdclk.logical.cdclk; - if (IS_GEMINILAKE(to_i915(intel_crtc->base.dev)) || - IS_CANNONLAKE(to_i915(intel_crtc->base.dev))) + if (HAS_2PPC(to_i915(intel_crtc->base.dev))) dotclk *= 2; pipe_max_pixel_rate = div_round_up_u32_fixed16(dotclk, pipe_downscale); -- 2.13.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/i915: Introduce HAS_2PPC. 2017-08-17 0:54 ` [PATCH 2/2] drm/i915: Introduce HAS_2PPC Rodrigo Vivi @ 2017-08-30 19:12 ` Pandiyan, Dhinakaran 2017-08-30 19:32 ` Chris Wilson 0 siblings, 1 reply; 10+ messages in thread From: Pandiyan, Dhinakaran @ 2017-08-30 19:12 UTC (permalink / raw) To: Vivi, Rodrigo; +Cc: intel-gfx, Zanoni, Paulo R On Wed, 2017-08-16 at 17:54 -0700, Rodrigo Vivi wrote: > Let's make it easier to add platforms that supports 2 pixel per > clock. > > With spread checks per platform it was easy to miss one or > another spot leading to loose some time on debug. > > Hopefully this check would save some cases in the future. > > No functional change. > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 4 ++++ > drivers/gpu/drm/i915/i915_pci.c | 2 ++ > drivers/gpu/drm/i915/intel_cdclk.c | 8 ++++---- > drivers/gpu/drm/i915/intel_pm.c | 3 +-- > 4 files changed, 11 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 6c25c8520c87..94f5e6522e5e 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -748,6 +748,7 @@ struct intel_csr { > func(is_lp); \ > func(is_alpha_support); \ > /* Keep has_* in alphabetical order */ \ > + func(has_2ppc); \ > func(has_64bit_reloc); \ > func(has_aliasing_ppgtt); \ > func(has_csr); \ > @@ -3025,6 +3026,9 @@ intel_info(const struct drm_i915_private *dev_priv) > #define NEEDS_WaRsDisableCoarsePowerGating(dev_priv) \ > (IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv)) > > +/* Supports 2 pixel per clock */ > +#define HAS_2PPC(dev_priv) ((dev_priv)->info.has_2ppc) > + How about #define HAS_2PPC(dev_priv) (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10) ? I am not clear on what qualifies for a place in device_info, but defining it this way let's me go to the definition and quickly check which platform has 2 pixels per clock. But again, iirc we won't need this with Ville's changes merged. > /* > * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts > * even when in MSI mode. This results in spurious interrupt warnings if the > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index 09d97e0990b7..df84025579cf 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -405,6 +405,7 @@ static const struct intel_device_info intel_geminilake_info = { > GEN9_LP_FEATURES, > .platform = INTEL_GEMINILAKE, > .ddb_size = 1024, > + .has_2ppc = 1, > .color = { .degamma_lut_size = 0, .gamma_lut_size = 1024 } > }; > > @@ -450,6 +451,7 @@ static const struct intel_device_info intel_cannonlake_info = { > .gen = 10, > .ddb_size = 1024, > .has_csr = 1, > + .has_2ppc = 1, > .color = { .degamma_lut_size = 0, .gamma_lut_size = 1024 } > }; > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > index 6b1d805fb755..edbccda40f0c 100644 > --- a/drivers/gpu/drm/i915/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > @@ -1752,7 +1752,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state, > crtc_state->has_audio && > crtc_state->port_clock >= 540000 && > crtc_state->lane_count == 4) { > - if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) > + if (HAS_2PPC(dev_priv)) > pixel_rate = max(2 * 316800, pixel_rate); > else > pixel_rate = max(432000, pixel_rate); > @@ -1764,7 +1764,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state, > * two pixels per clock. > */ > if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) { > - if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) > + if (HAS_2PPC(dev_priv)) > pixel_rate = max(2 * 2 * 96000, pixel_rate); > else > pixel_rate = max(2 * 96000, pixel_rate); > @@ -1997,14 +1997,14 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv) > { > int max_cdclk_freq = dev_priv->max_cdclk_freq; > > - if (IS_CANNONLAKE(dev_priv)) > - return 2 * max_cdclk_freq; > if (IS_GEMINILAKE(dev_priv)) > /* > * FIXME: Limiting to 99% as a temporary workaround. See > * glk_calc_cdclk() for details. > */ > return 2 * max_cdclk_freq * 99 / 100; > + else if (HAS_2PPC(dev_priv)) > + return 2 * max_cdclk_freq; > else if (INTEL_INFO(dev_priv)->gen >= 9 || > IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > return max_cdclk_freq; > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 42f753df30cb..c8da6ca4e8df 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3969,8 +3969,7 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc, > crtc_clock = crtc_state->adjusted_mode.crtc_clock; > dotclk = to_intel_atomic_state(state)->cdclk.logical.cdclk; > > - if (IS_GEMINILAKE(to_i915(intel_crtc->base.dev)) || > - IS_CANNONLAKE(to_i915(intel_crtc->base.dev))) > + if (HAS_2PPC(to_i915(intel_crtc->base.dev))) > dotclk *= 2; > > pipe_max_pixel_rate = div_round_up_u32_fixed16(dotclk, pipe_downscale); _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/i915: Introduce HAS_2PPC. 2017-08-30 19:12 ` Pandiyan, Dhinakaran @ 2017-08-30 19:32 ` Chris Wilson 2017-08-31 2:58 ` Pandiyan, Dhinakaran 0 siblings, 1 reply; 10+ messages in thread From: Chris Wilson @ 2017-08-30 19:32 UTC (permalink / raw) To: Pandiyan, Dhinakaran, Vivi, Rodrigo; +Cc: intel-gfx, Zanoni, Paulo R Quoting Pandiyan, Dhinakaran (2017-08-30 20:12:52) > On Wed, 2017-08-16 at 17:54 -0700, Rodrigo Vivi wrote: > > Let's make it easier to add platforms that supports 2 pixel per > > clock. > > > > With spread checks per platform it was easy to miss one or > > another spot leading to loose some time on debug. > > > > Hopefully this check would save some cases in the future. > > > > No functional change. > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 4 ++++ > > drivers/gpu/drm/i915/i915_pci.c | 2 ++ > > drivers/gpu/drm/i915/intel_cdclk.c | 8 ++++---- > > drivers/gpu/drm/i915/intel_pm.c | 3 +-- > > 4 files changed, 11 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 6c25c8520c87..94f5e6522e5e 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -748,6 +748,7 @@ struct intel_csr { > > func(is_lp); \ > > func(is_alpha_support); \ > > /* Keep has_* in alphabetical order */ \ > > + func(has_2ppc); \ > > func(has_64bit_reloc); \ > > func(has_aliasing_ppgtt); \ > > func(has_csr); \ > > @@ -3025,6 +3026,9 @@ intel_info(const struct drm_i915_private *dev_priv) > > #define NEEDS_WaRsDisableCoarsePowerGating(dev_priv) \ > > (IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv)) > > > > +/* Supports 2 pixel per clock */ > > +#define HAS_2PPC(dev_priv) ((dev_priv)->info.has_2ppc) > > + > > How about > #define HAS_2PPC(dev_priv) (IS_GEMINILAKE(dev_priv) || > INTEL_GEN(dev_priv) >= 10) ? > > I am not clear on what qualifies for a place in device_info, but > defining it this way let's me go to the definition and quickly check > which platform has 2 pixels per clock. A couple of rules of thumb for starting with: Use device_info if: - it fundamentally changes how the device operates, such that knowing about it in debug logs is a key means of triage - number of branches x callsites > 8 (some estimate of the cost of inclusion inside device_info vs savings in object code, for a more realistic estimate a branch will ~12 bytes (depending on the phase of the moon) and cost for device info will be the addition of a few strings, and a couple of calls to use those string, so at a guess 100 bytes.) -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/i915: Introduce HAS_2PPC. 2017-08-30 19:32 ` Chris Wilson @ 2017-08-31 2:58 ` Pandiyan, Dhinakaran 2017-08-31 4:40 ` Rodrigo Vivi 0 siblings, 1 reply; 10+ messages in thread From: Pandiyan, Dhinakaran @ 2017-08-31 2:58 UTC (permalink / raw) To: chris; +Cc: intel-gfx, Zanoni, Paulo R, Vivi, Rodrigo On Wed, 2017-08-30 at 20:32 +0100, Chris Wilson wrote: > Quoting Pandiyan, Dhinakaran (2017-08-30 20:12:52) > > On Wed, 2017-08-16 at 17:54 -0700, Rodrigo Vivi wrote: > > > Let's make it easier to add platforms that supports 2 pixel per > > > clock. > > > > > > With spread checks per platform it was easy to miss one or > > > another spot leading to loose some time on debug. > > > > > > Hopefully this check would save some cases in the future. > > > > > > No functional change. > > > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_drv.h | 4 ++++ > > > drivers/gpu/drm/i915/i915_pci.c | 2 ++ > > > drivers/gpu/drm/i915/intel_cdclk.c | 8 ++++---- > > > drivers/gpu/drm/i915/intel_pm.c | 3 +-- > > > 4 files changed, 11 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > > index 6c25c8520c87..94f5e6522e5e 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -748,6 +748,7 @@ struct intel_csr { > > > func(is_lp); \ > > > func(is_alpha_support); \ > > > /* Keep has_* in alphabetical order */ \ > > > + func(has_2ppc); \ > > > func(has_64bit_reloc); \ > > > func(has_aliasing_ppgtt); \ > > > func(has_csr); \ > > > @@ -3025,6 +3026,9 @@ intel_info(const struct drm_i915_private *dev_priv) > > > #define NEEDS_WaRsDisableCoarsePowerGating(dev_priv) \ > > > (IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv)) > > > > > > +/* Supports 2 pixel per clock */ > > > +#define HAS_2PPC(dev_priv) ((dev_priv)->info.has_2ppc) > > > + > > > > How about > > #define HAS_2PPC(dev_priv) (IS_GEMINILAKE(dev_priv) || > > INTEL_GEN(dev_priv) >= 10) ? > > > > I am not clear on what qualifies for a place in device_info, but > > defining it this way let's me go to the definition and quickly check > > which platform has 2 pixels per clock. > > A couple of rules of thumb for starting with: > > Use device_info if: > > - it fundamentally changes how the device operates, such that knowing > about it in debug logs is a key means of triage > > - number of branches x callsites > 8 > (some estimate of the cost of inclusion inside device_info vs > savings in object code, for a more realistic estimate a branch will > ~12 bytes (depending on the phase of the moon) and cost for device > info will be the addition of a few strings, and a couple of calls > to use those string, so at a guess 100 bytes.) That's an interesting back-of-the-envelope calculation. I suppose the compiler is also more likely to load device_info->gen into a register than device_info->has_2ppc. > > -Chris > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/i915: Introduce HAS_2PPC. 2017-08-31 2:58 ` Pandiyan, Dhinakaran @ 2017-08-31 4:40 ` Rodrigo Vivi 2017-08-31 7:59 ` Daniel Vetter 0 siblings, 1 reply; 10+ messages in thread From: Rodrigo Vivi @ 2017-08-31 4:40 UTC (permalink / raw) To: Pandiyan, Dhinakaran; +Cc: intel-gfx, Vivi, Rodrigo, Zanoni, Paulo R On Wed, Aug 30, 2017 at 7:58 PM, Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com> wrote: > On Wed, 2017-08-30 at 20:32 +0100, Chris Wilson wrote: >> Quoting Pandiyan, Dhinakaran (2017-08-30 20:12:52) >> > On Wed, 2017-08-16 at 17:54 -0700, Rodrigo Vivi wrote: >> > > Let's make it easier to add platforms that supports 2 pixel per >> > > clock. >> > > >> > > With spread checks per platform it was easy to miss one or >> > > another spot leading to loose some time on debug. >> > > >> > > Hopefully this check would save some cases in the future. >> > > >> > > No functional change. >> > > >> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> >> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> >> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> >> > > --- >> > > drivers/gpu/drm/i915/i915_drv.h | 4 ++++ >> > > drivers/gpu/drm/i915/i915_pci.c | 2 ++ >> > > drivers/gpu/drm/i915/intel_cdclk.c | 8 ++++---- >> > > drivers/gpu/drm/i915/intel_pm.c | 3 +-- >> > > 4 files changed, 11 insertions(+), 6 deletions(-) >> > > >> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> > > index 6c25c8520c87..94f5e6522e5e 100644 >> > > --- a/drivers/gpu/drm/i915/i915_drv.h >> > > +++ b/drivers/gpu/drm/i915/i915_drv.h >> > > @@ -748,6 +748,7 @@ struct intel_csr { >> > > func(is_lp); \ >> > > func(is_alpha_support); \ >> > > /* Keep has_* in alphabetical order */ \ >> > > + func(has_2ppc); \ >> > > func(has_64bit_reloc); \ >> > > func(has_aliasing_ppgtt); \ >> > > func(has_csr); \ >> > > @@ -3025,6 +3026,9 @@ intel_info(const struct drm_i915_private *dev_priv) >> > > #define NEEDS_WaRsDisableCoarsePowerGating(dev_priv) \ >> > > (IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv)) >> > > >> > > +/* Supports 2 pixel per clock */ >> > > +#define HAS_2PPC(dev_priv) ((dev_priv)->info.has_2ppc) >> > > + >> > >> > How about >> > #define HAS_2PPC(dev_priv) (IS_GEMINILAKE(dev_priv) || >> > INTEL_GEN(dev_priv) >= 10) ? >> > >> > I am not clear on what qualifies for a place in device_info, but >> > defining it this way let's me go to the definition and quickly check >> > which platform has 2 pixels per clock. One thing I always wanted was to avoid 2 places to declare features & capabilities. Here or on the platform. >> >> A couple of rules of thumb for starting with: >> >> Use device_info if: >> >> - it fundamentally changes how the device operates, such that knowing >> about it in debug logs is a key means of triage I think on this one 2ppc qualifies >> >> - number of branches x callsites > 8 2 * 5 > 8 in this case? or I'm getting the number of "branches" incorrectly? What I was trying to save as well is the addition of any next platform. When adding the feature it would be easier to search for HAS_2PPC instead of searching for glk or cnl and analising that individually to see if it is 2pp before see if it applies to that platform. But the reason that I put this patch on top is that I don't have strong opinion here so t would be fine for me to move on without ht. Thanks, Rodrigo. >> (some estimate of the cost of inclusion inside device_info vs >> savings in object code, for a more realistic estimate a branch will >> ~12 bytes (depending on the phase of the moon) and cost for device >> info will be the addition of a few strings, and a couple of calls >> to use those string, so at a guess 100 bytes.) > > That's an interesting back-of-the-envelope calculation. I suppose the > compiler is also more likely to load device_info->gen into a register > than device_info->has_2ppc. > > >> >> -Chris >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/i915: Introduce HAS_2PPC. 2017-08-31 4:40 ` Rodrigo Vivi @ 2017-08-31 7:59 ` Daniel Vetter 0 siblings, 0 replies; 10+ messages in thread From: Daniel Vetter @ 2017-08-31 7:59 UTC (permalink / raw) To: Rodrigo Vivi Cc: Zanoni, Paulo R, intel-gfx, Pandiyan, Dhinakaran, Vivi, Rodrigo On Wed, Aug 30, 2017 at 09:40:06PM -0700, Rodrigo Vivi wrote: > On Wed, Aug 30, 2017 at 7:58 PM, Pandiyan, Dhinakaran > <dhinakaran.pandiyan@intel.com> wrote: > > On Wed, 2017-08-30 at 20:32 +0100, Chris Wilson wrote: > >> Quoting Pandiyan, Dhinakaran (2017-08-30 20:12:52) > >> > On Wed, 2017-08-16 at 17:54 -0700, Rodrigo Vivi wrote: > >> > > Let's make it easier to add platforms that supports 2 pixel per > >> > > clock. > >> > > > >> > > With spread checks per platform it was easy to miss one or > >> > > another spot leading to loose some time on debug. > >> > > > >> > > Hopefully this check would save some cases in the future. > >> > > > >> > > No functional change. > >> > > > >> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > >> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > >> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > >> > > --- > >> > > drivers/gpu/drm/i915/i915_drv.h | 4 ++++ > >> > > drivers/gpu/drm/i915/i915_pci.c | 2 ++ > >> > > drivers/gpu/drm/i915/intel_cdclk.c | 8 ++++---- > >> > > drivers/gpu/drm/i915/intel_pm.c | 3 +-- > >> > > 4 files changed, 11 insertions(+), 6 deletions(-) > >> > > > >> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > >> > > index 6c25c8520c87..94f5e6522e5e 100644 > >> > > --- a/drivers/gpu/drm/i915/i915_drv.h > >> > > +++ b/drivers/gpu/drm/i915/i915_drv.h > >> > > @@ -748,6 +748,7 @@ struct intel_csr { > >> > > func(is_lp); \ > >> > > func(is_alpha_support); \ > >> > > /* Keep has_* in alphabetical order */ \ > >> > > + func(has_2ppc); \ > >> > > func(has_64bit_reloc); \ > >> > > func(has_aliasing_ppgtt); \ > >> > > func(has_csr); \ > >> > > @@ -3025,6 +3026,9 @@ intel_info(const struct drm_i915_private *dev_priv) > >> > > #define NEEDS_WaRsDisableCoarsePowerGating(dev_priv) \ > >> > > (IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv)) > >> > > > >> > > +/* Supports 2 pixel per clock */ > >> > > +#define HAS_2PPC(dev_priv) ((dev_priv)->info.has_2ppc) > >> > > + > >> > > >> > How about > >> > #define HAS_2PPC(dev_priv) (IS_GEMINILAKE(dev_priv) || > >> > INTEL_GEN(dev_priv) >= 10) ? > >> > > >> > I am not clear on what qualifies for a place in device_info, but > >> > defining it this way let's me go to the definition and quickly check > >> > which platform has 2 pixels per clock. > > One thing I always wanted was to avoid 2 places to declare features & > capabilities. > Here or on the platform. So should we do a bit of extraction to bring the two closer together? I.e. h/c file with only the device info and feature macros, nothing else. We still need the split, but well we need the structure in a header anyway. If you actually want to bring them closer together, then you need to bring them closer together. Not arbitrarily prefer one over the other (while still having piles of other users around of the non-preferred version). > > >> > >> A couple of rules of thumb for starting with: > >> > >> Use device_info if: > >> > >> - it fundamentally changes how the device operates, such that knowing > >> about it in debug logs is a key means of triage > > I think on this one 2ppc qualifies gen3 did double-clock too, just fyi :-) We survived just fine without a feature flag on that. Anyway, just my bikeshed. > >> > >> - number of branches x callsites > 8 > > 2 * 5 > 8 in this case? > or I'm getting the number of "branches" incorrectly? > > What I was trying to save as well is the addition of any next platform. > When adding the feature it would be easier to search for HAS_2PPC instead of > searching for glk or cnl and analising that individually to see if it > is 2pp before > see if it applies to that platform. > > But the reason that I put this patch on top is that I don't have > strong opinion here so > t would be fine for me to move on without ht. HAS_2PPC definitely makes sense, but I'm not sure it makes sense to stuff everything into intel_device_info. Going that way some people might start to think that this is the complete solution to describing everything, and that definitely doesn't work. -Daniel > Thanks, > Rodrigo. > > >> (some estimate of the cost of inclusion inside device_info vs > >> savings in object code, for a more realistic estimate a branch will > >> ~12 bytes (depending on the phase of the moon) and cost for device > >> info will be the addition of a few strings, and a couple of calls > >> to use those string, so at a guess 100 bytes.) > > > > That's an interesting back-of-the-envelope calculation. I suppose the > > compiler is also more likely to load device_info->gen into a register > > than device_info->has_2ppc. > > > > > >> > >> -Chris > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Rodrigo Vivi > Blog: http://blog.vivi.eng.br > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/cnl: Allow 2 pixel per clock on Cannonlake. 2017-08-17 0:54 [PATCH 1/2] drm/i915/cnl: Allow 2 pixel per clock on Cannonlake Rodrigo Vivi 2017-08-17 0:54 ` [PATCH 2/2] drm/i915: Introduce HAS_2PPC Rodrigo Vivi @ 2017-08-17 1:28 ` Patchwork 2017-08-30 18:14 ` [PATCH 1/2] " Pandiyan, Dhinakaran 2 siblings, 0 replies; 10+ messages in thread From: Patchwork @ 2017-08-17 1:28 UTC (permalink / raw) To: Rodrigo Vivi; +Cc: intel-gfx == Series Details == Series: series starting with [1/2] drm/i915/cnl: Allow 2 pixel per clock on Cannonlake. URL : https://patchwork.freedesktop.org/series/28890/ State : success == Summary == Series 28890v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/28890/revisions/1/mbox/ Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-b: pass -> DMESG-WARN (fi-byt-n2820) fdo#101705 fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:453s fi-bdw-gvtdvm total:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:441s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:356s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:552s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:519s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:525s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:514s fi-glk-2a total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:614s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:446s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:422s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:428s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:508s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:476s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:476s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:599s fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:598s fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:526s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:468s fi-skl-6700k total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:477s fi-skl-6770hq total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:489s fi-skl-gvtdvm total:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:444s fi-skl-x1585l total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:489s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:545s fi-snb-2600 total:279 pass:250 dwarn:0 dfail:0 fail:0 skip:29 time:410s ada53b43f81fe618f3f0f1dfbd3dd776bb277323 drm-tip: 2017y-08m-16d-15h-18m-56s UTC integration manifest 36dd9803628a drm/i915: Introduce HAS_2PPC. 82245cda4c5c drm/i915/cnl: Allow 2 pixel per clock on Cannonlake. == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5420/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] drm/i915/cnl: Allow 2 pixel per clock on Cannonlake. 2017-08-17 0:54 [PATCH 1/2] drm/i915/cnl: Allow 2 pixel per clock on Cannonlake Rodrigo Vivi 2017-08-17 0:54 ` [PATCH 2/2] drm/i915: Introduce HAS_2PPC Rodrigo Vivi 2017-08-17 1:28 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/cnl: Allow 2 pixel per clock on Cannonlake Patchwork @ 2017-08-30 18:14 ` Pandiyan, Dhinakaran 2017-08-30 18:59 ` Ville Syrjälä 2 siblings, 1 reply; 10+ messages in thread From: Pandiyan, Dhinakaran @ 2017-08-30 18:14 UTC (permalink / raw) To: Vivi, Rodrigo; +Cc: Nikula, Jani, intel-gfx On Wed, 2017-08-16 at 17:54 -0700, Rodrigo Vivi wrote: > This is heavily based on a initial patch provided by Ville > plus all changes provided later by Ander. > Ville abandoned this change last time stating - "Assume 1 pixel per clock for the purposes of max pixel rate calculation until DDI clock voltage scaling is handled" If you can confirm that change was implemented, this patch is Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Also, I see that Ville's patch to change the pixel rate checks to use cdclk has not been merged > As Geminilake, Cannonlake also supports 2 pixels per clock. > > Different from Geminilake we are not implementing the 99% Wa. > But we can revisit that decision later if we find out > any limitation on later CNL SKUs. > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Cc: Jani Nikula <jani.nikula@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/intel_cdclk.c | 12 ++++++------ > drivers/gpu/drm/i915/intel_pm.c | 3 ++- > 2 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > index 1241e5891b29..6b1d805fb755 100644 > --- a/drivers/gpu/drm/i915/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > @@ -1422,9 +1422,9 @@ void bxt_uninit_cdclk(struct drm_i915_private *dev_priv) > > static int cnl_calc_cdclk(int max_pixclk) > { > - if (max_pixclk > 336000) > + if (max_pixclk > 2 * 336000) > return 528000; > - else if (max_pixclk > 168000) > + else if (max_pixclk > 2 * 168000) > return 336000; > else > return 168000; > @@ -1752,9 +1752,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state, > crtc_state->has_audio && > crtc_state->port_clock >= 540000 && > crtc_state->lane_count == 4) { > - if (IS_CANNONLAKE(dev_priv)) > - pixel_rate = max(316800, pixel_rate); > - else if (IS_GEMINILAKE(dev_priv)) > + if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) > pixel_rate = max(2 * 316800, pixel_rate); > else > pixel_rate = max(432000, pixel_rate); > @@ -1766,7 +1764,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state, > * two pixels per clock. > */ > if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) { > - if (IS_GEMINILAKE(dev_priv)) > + if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) > pixel_rate = max(2 * 2 * 96000, pixel_rate); > else > pixel_rate = max(2 * 96000, pixel_rate); > @@ -1999,6 +1997,8 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv) > { > int max_cdclk_freq = dev_priv->max_cdclk_freq; > > + if (IS_CANNONLAKE(dev_priv)) > + return 2 * max_cdclk_freq; > if (IS_GEMINILAKE(dev_priv)) > /* > * FIXME: Limiting to 99% as a temporary workaround. See > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index ed662937ec3c..42f753df30cb 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3969,7 +3969,8 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc, > crtc_clock = crtc_state->adjusted_mode.crtc_clock; > dotclk = to_intel_atomic_state(state)->cdclk.logical.cdclk; > > - if (IS_GEMINILAKE(to_i915(intel_crtc->base.dev))) > + if (IS_GEMINILAKE(to_i915(intel_crtc->base.dev)) || > + IS_CANNONLAKE(to_i915(intel_crtc->base.dev))) > dotclk *= 2; > > pipe_max_pixel_rate = div_round_up_u32_fixed16(dotclk, pipe_downscale); _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] drm/i915/cnl: Allow 2 pixel per clock on Cannonlake. 2017-08-30 18:14 ` [PATCH 1/2] " Pandiyan, Dhinakaran @ 2017-08-30 18:59 ` Ville Syrjälä 0 siblings, 0 replies; 10+ messages in thread From: Ville Syrjälä @ 2017-08-30 18:59 UTC (permalink / raw) To: Pandiyan, Dhinakaran; +Cc: Nikula, Jani, intel-gfx, Vivi, Rodrigo On Wed, Aug 30, 2017 at 06:14:03PM +0000, Pandiyan, Dhinakaran wrote: > > On Wed, 2017-08-16 at 17:54 -0700, Rodrigo Vivi wrote: > > This is heavily based on a initial patch provided by Ville > > plus all changes provided later by Ander. > > > > Ville abandoned this change last time stating - "Assume 1 pixel per > clock for the purposes of max pixel rate calculation until DDI clock > voltage scaling is handled" > > If you can confirm that change was implemented, this patch is > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > > Also, I see that Ville's patch to change the pixel rate checks to use > cdclk has not been merged I totally forgot about that one TBH. I've just reposted first patch to get a fresh CI run for it. I think this CNL thing might look cleaner if redone on top of my stuff, but if people want this in ASAP I can rebase my stuff as well. > > > > > As Geminilake, Cannonlake also supports 2 pixels per clock. > > > > Different from Geminilake we are not implementing the 99% Wa. > > But we can revisit that decision later if we find out > > any limitation on later CNL SKUs. > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > Cc: Jani Nikula <jani.nikula@intel.com> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > --- > > drivers/gpu/drm/i915/intel_cdclk.c | 12 ++++++------ > > drivers/gpu/drm/i915/intel_pm.c | 3 ++- > > 2 files changed, 8 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > > index 1241e5891b29..6b1d805fb755 100644 > > --- a/drivers/gpu/drm/i915/intel_cdclk.c > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > > @@ -1422,9 +1422,9 @@ void bxt_uninit_cdclk(struct drm_i915_private *dev_priv) > > > > static int cnl_calc_cdclk(int max_pixclk) > > { > > - if (max_pixclk > 336000) > > + if (max_pixclk > 2 * 336000) > > return 528000; > > - else if (max_pixclk > 168000) > > + else if (max_pixclk > 2 * 168000) > > return 336000; > > else > > return 168000; > > @@ -1752,9 +1752,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state, > > crtc_state->has_audio && > > crtc_state->port_clock >= 540000 && > > crtc_state->lane_count == 4) { > > - if (IS_CANNONLAKE(dev_priv)) > > - pixel_rate = max(316800, pixel_rate); > > - else if (IS_GEMINILAKE(dev_priv)) > > + if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) > > pixel_rate = max(2 * 316800, pixel_rate); > > else > > pixel_rate = max(432000, pixel_rate); > > @@ -1766,7 +1764,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state, > > * two pixels per clock. > > */ > > if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) { > > - if (IS_GEMINILAKE(dev_priv)) > > + if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) > > pixel_rate = max(2 * 2 * 96000, pixel_rate); > > else > > pixel_rate = max(2 * 96000, pixel_rate); > > @@ -1999,6 +1997,8 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv) > > { > > int max_cdclk_freq = dev_priv->max_cdclk_freq; > > > > + if (IS_CANNONLAKE(dev_priv)) > > + return 2 * max_cdclk_freq; > > if (IS_GEMINILAKE(dev_priv)) > > /* > > * FIXME: Limiting to 99% as a temporary workaround. See > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index ed662937ec3c..42f753df30cb 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -3969,7 +3969,8 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc, > > crtc_clock = crtc_state->adjusted_mode.crtc_clock; > > dotclk = to_intel_atomic_state(state)->cdclk.logical.cdclk; > > > > - if (IS_GEMINILAKE(to_i915(intel_crtc->base.dev))) > > + if (IS_GEMINILAKE(to_i915(intel_crtc->base.dev)) || > > + IS_CANNONLAKE(to_i915(intel_crtc->base.dev))) > > dotclk *= 2; > > > > pipe_max_pixel_rate = div_round_up_u32_fixed16(dotclk, pipe_downscale); -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-08-31 7:59 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-08-17 0:54 [PATCH 1/2] drm/i915/cnl: Allow 2 pixel per clock on Cannonlake Rodrigo Vivi 2017-08-17 0:54 ` [PATCH 2/2] drm/i915: Introduce HAS_2PPC Rodrigo Vivi 2017-08-30 19:12 ` Pandiyan, Dhinakaran 2017-08-30 19:32 ` Chris Wilson 2017-08-31 2:58 ` Pandiyan, Dhinakaran 2017-08-31 4:40 ` Rodrigo Vivi 2017-08-31 7:59 ` Daniel Vetter 2017-08-17 1:28 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/cnl: Allow 2 pixel per clock on Cannonlake Patchwork 2017-08-30 18:14 ` [PATCH 1/2] " Pandiyan, Dhinakaran 2017-08-30 18:59 ` Ville Syrjälä
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.