From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2DD89C28CBC for ; Wed, 6 May 2020 12:22:38 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 0093F206DB for ; Wed, 6 May 2020 12:22:37 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0093F206DB Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 768828934B; Wed, 6 May 2020 12:22:37 +0000 (UTC) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3CC4C8934B for ; Wed, 6 May 2020 12:22:36 +0000 (UTC) IronPort-SDR: rb4M3Z6e9WvdjvqN9aaLvTL5qYD1xz9qeUSuWLCKpe5omi1KkIqcvQlIbyj8Znc/KcY52cwPWk 12WECmBorBlg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 May 2020 05:22:35 -0700 IronPort-SDR: e7kLRt7yqrWiZsTQ9mlbXSqthQWPl41eIAxXlGyjqQVAR6AgbmmMnynBlZFldQZbuq7ehlJzHS pWbld8p9NT+Q== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,358,1583222400"; d="scan'208";a="248900725" Received: from genxfsim-desktop.iind.intel.com (HELO intel.com) ([10.223.74.178]) by orsmga007.jf.intel.com with ESMTP; 06 May 2020 05:22:33 -0700 Date: Wed, 6 May 2020 17:43:18 +0530 From: Anshuman Gupta To: Imre Deak Message-ID: <20200506121317.GE10565@intel.com> References: <20200504225227.464666-1-matthew.d.roper@intel.com> <20200504225227.464666-9-matthew.d.roper@intel.com> <20200505045058.GD10565@intel.com> <20200505143904.GQ188376@mdroper-desk1.amr.corp.intel.com> <20200505160954.GC20861@ideak-desk.fi.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200505160954.GC20861@ideak-desk.fi.intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Subject: Re: [Intel-gfx] [PATCH v2 08/22] drm/i915/rkl: Add power well support X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Lucas De Marchi , intel-gfx@lists.freedesktop.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On 2020-05-05 at 19:09:54 +0300, Imre Deak wrote: > On Tue, May 05, 2020 at 07:39:04AM -0700, Matt Roper wrote: > > On Tue, May 05, 2020 at 10:20:58AM +0530, Anshuman Gupta wrote: > > > On 2020-05-04 at 15:52:13 -0700, Matt Roper wrote: > > > > RKL power wells are similar to TGL power wells, but have some important > > > > differences: > > > > > > > > * PG1 now has pipe A's VDSC (rather than sticking it in PG2) > > > > * PG2 no longer exists > > > > * DDI-C (aka TC-1) moves from PG1 -> PG3 > > > > * PG5 no longer exists due to the lack of a fourth pipe > > > > > > > > Also note that what we refer to as 'DDI-C' and 'DDI-D' need to actually > > > > be programmed as TC-1 and TC-2 even though this platform doesn't have TC > > > > outputs. Looks good to me. Reviewed-by: Anshuman Gupta > > > > > > > > Bspec: 49234 > > > > Cc: Imre Deak > > > > Cc: Lucas De Marchi > > > > Cc: Anshuman Gupta > > > > Signed-off-by: Matt Roper > > > > --- > > > > .../drm/i915/display/intel_display_power.c | 185 +++++++++++++++++- > > > > drivers/gpu/drm/i915/display/intel_vdsc.c | 4 +- > > > > 2 files changed, 186 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > > > > index 49998906cc61..71691919d101 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > > > > @@ -2913,6 +2913,53 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, > > > > BIT_ULL(POWER_DOMAIN_AUX_I_TBT) | \ > > > > BIT_ULL(POWER_DOMAIN_TC_COLD_OFF)) > > > > > > > > +#define RKL_PW_4_POWER_DOMAINS ( \ > > > > + BIT_ULL(POWER_DOMAIN_PIPE_C) | \ > > > > + BIT_ULL(POWER_DOMAIN_PIPE_C_PANEL_FITTER) | \ > > > > + BIT_ULL(POWER_DOMAIN_TRANSCODER_C) | \ > > > > + BIT_ULL(POWER_DOMAIN_INIT)) > > > > + > > > > +#define RKL_PW_3_POWER_DOMAINS ( \ > > > > + RKL_PW_4_POWER_DOMAINS | \ > > > > + BIT_ULL(POWER_DOMAIN_PIPE_B) | \ > > > > + BIT_ULL(POWER_DOMAIN_PIPE_B_PANEL_FITTER) | \ > > > > + BIT_ULL(POWER_DOMAIN_AUDIO) | \ > > > > + BIT_ULL(POWER_DOMAIN_VGA) | \ > > > > + BIT_ULL(POWER_DOMAIN_TRANSCODER_B) | \ > > > > + BIT_ULL(POWER_DOMAIN_PORT_DDI_D_LANES) | \ > > > > + BIT_ULL(POWER_DOMAIN_PORT_DDI_E_LANES) | \ > > > > + BIT_ULL(POWER_DOMAIN_AUX_D) | \ > > > > + BIT_ULL(POWER_DOMAIN_AUX_E) | \ > > > > + BIT_ULL(POWER_DOMAIN_INIT)) > > > > + > > > > +/* > > > > + * There is no PW_2/PG_2 on RKL. > > > > + * > > > > + * RKL PW_1/PG_1 domains (under HW/DMC control): > > > > + * - DBUF function (note: registers are in PW0) > > > > + * - PIPE_A and its planes and VDSC/joining, except VGA > > > > + * - transcoder A > > > > + * - DDI_A and DDI_B > > > > + * - FBC > > > > + * > > > > + * RKL PW_0/PG_0 domains (under HW/DMC control): > > > > + * - PCI > > > > + * - clocks except port PLL > > > > + * - shared functions: > > > > + * * interrupts except pipe interrupts > > > > + * * MBus except PIPE_MBUS_DBOX_CTL > > > > + * * DBUF registers > > > > + * - central power except FBC > > > > + * - top-level GTC (DDI-level GTC is in the well associated with the DDI) > > > > + */ > > > > + > > > > +#define RKL_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > > > > + RKL_PW_3_POWER_DOMAINS | \ > > > > + BIT_ULL(POWER_DOMAIN_MODESET) | \ > > > > + BIT_ULL(POWER_DOMAIN_AUX_A) | \ > > > > + BIT_ULL(POWER_DOMAIN_AUX_B) | \ > > > > + BIT_ULL(POWER_DOMAIN_INIT)) > > > > + > > > > static const struct i915_power_well_ops i9xx_always_on_power_well_ops = { > > > > .sync_hw = i9xx_power_well_sync_hw_noop, > > > > .enable = i9xx_always_on_power_well_noop, > > > > @@ -4283,6 +4330,140 @@ static const struct i915_power_well_desc tgl_power_wells[] = { > > > > }, > > > > }; > > > > > > > > +static const struct i915_power_well_desc rkl_power_wells[] = { > > > > + { > > > > + .name = "always-on", > > > > + .always_on = true, > > > > + .domains = POWER_DOMAIN_MASK, > > > > + .ops = &i9xx_always_on_power_well_ops, > > > > + .id = DISP_PW_ID_NONE, > > > > + }, > > > > + { > > > > + .name = "power well 1", > > > > + /* Handled by the DMC firmware */ > > > > + .always_on = true, > > > > + .domains = 0, > > > > + .ops = &hsw_power_well_ops, > > > > + .id = SKL_DISP_PW_1, > > > > + { > > > > + .hsw.regs = &hsw_power_well_regs, > > > > + .hsw.idx = ICL_PW_CTL_IDX_PW_1, > > > > + .hsw.has_fuses = true, > > > > + }, > > > > + }, > > > > + { > > > > + .name = "DC off", > > > > + .domains = RKL_DISPLAY_DC_OFF_POWER_DOMAINS, > > > > + .ops = &gen9_dc_off_power_well_ops, > > > > + .id = SKL_DISP_DC_OFF, > > > > + }, > > > > + { > > > > + .name = "power well 3", > > > > + .domains = RKL_PW_3_POWER_DOMAINS, > > > > + .ops = &hsw_power_well_ops, > > > > + .id = ICL_DISP_PW_3, > > > > + { > > > > + .hsw.regs = &hsw_power_well_regs, > > > > + .hsw.idx = ICL_PW_CTL_IDX_PW_3, > > > > + .hsw.irq_pipe_mask = BIT(PIPE_B), > > > > + .hsw.has_vga = true, > > > > + .hsw.has_fuses = true, > > > > + }, > > > > + }, > > > > + { > > > > + .name = "power well 4", > > > > + .domains = RKL_PW_4_POWER_DOMAINS, > > > > + .ops = &hsw_power_well_ops, > > > > + .id = DISP_PW_ID_NONE, > > > > + { > > > > + .hsw.regs = &hsw_power_well_regs, > > > > + .hsw.idx = ICL_PW_CTL_IDX_PW_4, > > > > + .hsw.has_fuses = true, > > > > + .hsw.irq_pipe_mask = BIT(PIPE_C), > > > > + } > > > > + }, > > > IMHO the order of power well is differs from TGL/ICL, > > > TGL/ICL PW_4 comes after DDI and AUX IO power wells. > > > Why the order is different on RKL. > > > Thanks, > > > Anshuman Gupta. > > > > I'm not aware of any specific reason why PW_4 was placed after the > > DDI/AUX wells on ICL, but that order seems to have been copy/pasted to > > TGL too. > > > > It seems more natural to keep the main power wells together, so I wrote > > them in what I considered to be a more intuitive order here. But > > ultimately the order of the power wells in the array shouldn't have any > > functional impact. > > The order of two power wells in the array determines their > enabling/disabling order if they are included in the same power domain. > If there is no dependency between two power wells their order in the > array is not important. > > > Matt > > > > > > + { > > > > + .name = "DDI A IO", > > > > + .domains = ICL_DDI_IO_A_POWER_DOMAINS, > > > > + .ops = &hsw_power_well_ops, > > > > + .id = DISP_PW_ID_NONE, > > > > + { > > > > + .hsw.regs = &icl_ddi_power_well_regs, > > > > + .hsw.idx = ICL_PW_CTL_IDX_DDI_A, > > > > + } > > > > + }, > > > > + { > > > > + .name = "DDI B IO", > > > > + .domains = ICL_DDI_IO_B_POWER_DOMAINS, > > > > + .ops = &hsw_power_well_ops, > > > > + .id = DISP_PW_ID_NONE, > > > > + { > > > > + .hsw.regs = &icl_ddi_power_well_regs, > > > > + .hsw.idx = ICL_PW_CTL_IDX_DDI_B, > > > > + } > > > > + }, > > > > + { > > > > + .name = "DDI D TC1 IO", > > > > + .domains = TGL_DDI_IO_D_TC1_POWER_DOMAINS, > > > > + .ops = &hsw_power_well_ops, > > > > + .id = DISP_PW_ID_NONE, > > > > + { > > > > + .hsw.regs = &icl_ddi_power_well_regs, > > > > + .hsw.idx = TGL_PW_CTL_IDX_DDI_TC1, > > > > + }, > > > > + }, > > > > + { > > > > + .name = "DDI E TC2 IO", > > > > + .domains = TGL_DDI_IO_E_TC2_POWER_DOMAINS, > > > > + .ops = &hsw_power_well_ops, > > > > + .id = DISP_PW_ID_NONE, > > > > + { > > > > + .hsw.regs = &icl_ddi_power_well_regs, > > > > + .hsw.idx = TGL_PW_CTL_IDX_DDI_TC2, > > > > + }, > > > > + }, > > > > + { > > > > + .name = "AUX A", > > > > + .domains = ICL_AUX_A_IO_POWER_DOMAINS, > > > > + .ops = &icl_aux_power_well_ops, > > > > + .id = DISP_PW_ID_NONE, > > > > + { > > > > + .hsw.regs = &icl_aux_power_well_regs, > > > > + .hsw.idx = ICL_PW_CTL_IDX_AUX_A, > > > > + }, > > > > + }, > > > > + { > > > > + .name = "AUX B", > > > > + .domains = ICL_AUX_B_IO_POWER_DOMAINS, > > > > + .ops = &icl_aux_power_well_ops, > > > > + .id = DISP_PW_ID_NONE, > > > > + { > > > > + .hsw.regs = &icl_aux_power_well_regs, > > > > + .hsw.idx = ICL_PW_CTL_IDX_AUX_B, > > > > + }, > > > > + }, > > > > + { > > > > + .name = "AUX D TC1", > > > > + .domains = TGL_AUX_D_TC1_IO_POWER_DOMAINS, > > > > + .ops = &icl_aux_power_well_ops, > > > > + .id = DISP_PW_ID_NONE, > > > > + { > > > > + .hsw.regs = &icl_aux_power_well_regs, > > > > + .hsw.idx = TGL_PW_CTL_IDX_AUX_TC1, > > > > + }, > > > > + }, > > > > + { > > > > + .name = "AUX E TC2", > > > > + .domains = TGL_AUX_E_TC2_IO_POWER_DOMAINS, > > > > + .ops = &icl_aux_power_well_ops, > > > > + .id = DISP_PW_ID_NONE, > > > > + { > > > > + .hsw.regs = &icl_aux_power_well_regs, > > > > + .hsw.idx = TGL_PW_CTL_IDX_AUX_TC2, > > > > + }, > > > > + }, > > > > +}; > > > > + > > > > static int > > > > sanitize_disable_power_well_option(const struct drm_i915_private *dev_priv, > > > > int disable_power_well) > > > > @@ -4428,7 +4609,9 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv) > > > > * The enabling order will be from lower to higher indexed wells, > > > > * the disabling order is reversed. > > > > */ > > > > - if (IS_GEN(dev_priv, 12)) { > > > > + if (IS_ROCKETLAKE(dev_priv)) { > > > > + err = set_power_wells(power_domains, rkl_power_wells); > > > > + } else if (IS_GEN(dev_priv, 12)) { > > > > err = set_power_wells(power_domains, tgl_power_wells); > > > > } else if (IS_GEN(dev_priv, 11)) { > > > > err = set_power_wells(power_domains, icl_power_wells); > > > > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c > > > > index 95ad87d4ccb3..d145fe2bed81 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c > > > > @@ -476,13 +476,13 @@ intel_dsc_power_domain(const struct intel_crtc_state *crtc_state) > > > > * POWER_DOMAIN_TRANSCODER_VDSC_PW2 power domain in two cases: > > > > * > > > > * - ICL eDP/DSI transcoder > > > > - * - TGL pipe A > > > > + * - Gen12+ (except RKL) pipe A > > > > * > > > > * For any other pipe, VDSC/joining uses the power well associated with > > > > * the pipe in use. Hence another reference on the pipe power domain > > > > * will suffice. (Except no VDSC/joining on ICL pipe A.) > > > > */ > > > > - if (INTEL_GEN(i915) >= 12 && pipe == PIPE_A) > > > > + if (INTEL_GEN(i915) >= 12 && !IS_ROCKETLAKE(i915) && pipe == PIPE_A) > > > > return POWER_DOMAIN_TRANSCODER_VDSC_PW2; > > > > else if (is_pipe_dsc(crtc_state)) > > > > return POWER_DOMAIN_PIPE(pipe); > > > > -- > > > > 2.24.1 > > > > > > > > -- > > Matt Roper > > Graphics Software Engineer > > VTT-OSGC Platform Enablement > > Intel Corporation > > (916) 356-2795 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx