On Tue, Jan 23, 2018 at 02:43:22AM +0000, Pandiyan, Dhinakaran wrote:
>
>
>
> On Mon, 2018-01-22 at 15:59 -0800, Rodrigo Vivi wrote:
> > On some Cannonlake SKUs we have a dedicated Aux for port F,
> > that is only the full split between port A and port E.
> >
> > There is still no Aux E for Port E, as in previous platforms,
> > because port_E still means shared lanes with port A.
> >
> > v2: Rebase.
> > v3: Add couple missed PORT_F cases on intel_dp.
> > v4: Rebase and fix commit message.
> > v5: Squash Imre's "drm/i915: Add missing AUX_F power well string"
> > v6: Rebase on top of display headers rework.
> > v7: s/IS_CANNONLAKE/IS_CNL_WITH_PORT_F (DK)
> > v8: Fix Aux bits for Port F (DK)
> >
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandi...@intel.com>
> > Cc: Lucas De Marchi <lucas.demar...@intel.com>
> > Cc: Imre Deak <imre.d...@intel.com>
> > Cc: Manasi Navare <manasi.d.nav...@intel.com>
> > Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.v...@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > drivers/gpu/drm/i915/i915_irq.c | 6 ++++++
> > drivers/gpu/drm/i915/i915_reg.h | 9 +++++++++
> > drivers/gpu/drm/i915/intel_display.h | 1 +
> > drivers/gpu/drm/i915/intel_dp.c | 8 ++++++++
> > drivers/gpu/drm/i915/intel_runtime_pm.c | 11 +++++++++++
> > 6 files changed, 36 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 3d3727829ac7..7206c7c5f81c 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1255,6 +1255,7 @@ enum modeset_restore {
> > #define DP_AUX_B 0x10
> > #define DP_AUX_C 0x20
> > #define DP_AUX_D 0x30
> > +#define DP_AUX_F 0x50
>
> How is this decided? Looks like drivers/gpu/drm/i915/gvt/opregion.c
> <<virt_vbt_generation>> needs to be updated too. I guess that's a
> separate patch.
Thanks! Another thing that was totally wrong here...
It is defined by VBT.
"Block 2 (General Bytes Definition)"
Aux Channel: "0x60 = DisplayPort Aux F"
>
> >
> > #define DDC_PIN_B 0x05
> > #define DDC_PIN_C 0x04
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index db3466ec6faa..0af970d4b3cf 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2579,6 +2579,9 @@ gen8_de_irq_handler(struct drm_i915_private
> > *dev_priv, u32 master_ctl)
> > GEN9_AUX_CHANNEL_C |
> > GEN9_AUX_CHANNEL_D;
> >
> > + if (IS_CNL_WITH_PORT_F(dev_priv))
> > + tmp_mask |= CNL_AUX_CHANNEL_F;
> > +
> > if (iir & tmp_mask) {
> > dp_aux_irq_handler(dev_priv);
> > found = true;
> > @@ -3617,6 +3620,9 @@ static void gen8_de_irq_postinstall(struct
> > drm_i915_private *dev_priv)
> > de_pipe_masked |= GEN8_DE_PIPE_IRQ_FAULT_ERRORS;
> > }
> >
> > + if (IS_CNL_WITH_PORT_F(dev_priv))
> > + de_port_masked |= CNL_AUX_CHANNEL_F;
> > +
> > de_pipe_enables = de_pipe_masked | GEN8_PIPE_VBLANK |
> > GEN8_PIPE_FIFO_UNDERRUN;
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index abd9ee876186..ebdee212767a 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -1312,6 +1312,7 @@ enum i915_power_well_id {
> > CNL_DISP_PW_AUX_B = GLK_DISP_PW_AUX_B,
> > CNL_DISP_PW_AUX_C = GLK_DISP_PW_AUX_C,
> > CNL_DISP_PW_AUX_D,
> > + CNL_DISP_PW_AUX_F,
> >
> > SKL_DISP_PW_1 = 14,
> > SKL_DISP_PW_2,
> > @@ -5284,6 +5285,13 @@ enum {
> > #define _DPD_AUX_CH_DATA4 (dev_priv->info.display_mmio_offset + 0x64320)
> > #define _DPD_AUX_CH_DATA5 (dev_priv->info.display_mmio_offset + 0x64324)
> >
> > +#define _DPF_AUX_CH_CTL (dev_priv->info.display_mmio_offset +
> > 0x64510)
> > +#define _DPF_AUX_CH_DATA1 (dev_priv->info.display_mmio_offset + 0x64514)
> > +#define _DPF_AUX_CH_DATA2 (dev_priv->info.display_mmio_offset + 0x64518)
> > +#define _DPF_AUX_CH_DATA3 (dev_priv->info.display_mmio_offset + 0x6451c)
> > +#define _DPF_AUX_CH_DATA4 (dev_priv->info.display_mmio_offset + 0x64520)
> > +#define _DPF_AUX_CH_DATA5 (dev_priv->info.display_mmio_offset + 0x64524)
> > +
> > #define DP_AUX_CH_CTL(port) _MMIO_PORT(port, _DPA_AUX_CH_CTL,
> > _DPB_AUX_CH_CTL)
> > #define DP_AUX_CH_DATA(port, i) _MMIO(_PORT(port, _DPA_AUX_CH_DATA1,
> > _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
> >
> > @@ -6939,6 +6947,7 @@ enum {
> > #define GEN8_DE_PORT_IMR _MMIO(0x44444)
> > #define GEN8_DE_PORT_IIR _MMIO(0x44448)
> > #define GEN8_DE_PORT_IER _MMIO(0x4444c)
> > +#define CNL_AUX_CHANNEL_F (1 << 28)
> > #define GEN9_AUX_CHANNEL_D (1 << 27)
> > #define GEN9_AUX_CHANNEL_C (1 << 26)
> > #define GEN9_AUX_CHANNEL_B (1 << 25)
> > diff --git a/drivers/gpu/drm/i915/intel_display.h
> > b/drivers/gpu/drm/i915/intel_display.h
> > index e47638931b51..30fa2041a45f 100644
> > --- a/drivers/gpu/drm/i915/intel_display.h
> > +++ b/drivers/gpu/drm/i915/intel_display.h
> > @@ -172,6 +172,7 @@ enum intel_display_power_domain {
> > POWER_DOMAIN_AUX_B,
> > POWER_DOMAIN_AUX_C,
> > POWER_DOMAIN_AUX_D,
> > + POWER_DOMAIN_AUX_F,
> > POWER_DOMAIN_GMBUS,
> > POWER_DOMAIN_MODESET,
> > POWER_DOMAIN_GT_IRQ,
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index a2e887999915..ae3b0b030177 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1323,6 +1323,9 @@ static enum port intel_aux_port(struct
> > drm_i915_private *dev_priv,
> > case DP_AUX_D:
> > aux_port = PORT_D;
> > break;
> > + case DP_AUX_F:
> > + aux_port = PORT_F;
> > + break;
> > default:
> > MISSING_CASE(info->alternate_aux_channel);
> > aux_port = PORT_A;
> > @@ -1342,6 +1345,7 @@ static i915_reg_t g4x_aux_ctl_reg(struct
> > drm_i915_private *dev_priv,
> > case PORT_B:
> > case PORT_C:
> > case PORT_D:
> > + case PORT_F:
>
> This hunk and the next are not needed. skl_aux_ctl_reg() and
> skl_aux_data_reg() already have the required change.
>
> > return DP_AUX_CH_CTL(port);
> > default:
> > MISSING_CASE(port);
> > @@ -1356,6 +1360,7 @@ static i915_reg_t g4x_aux_data_reg(struct
> > drm_i915_private *dev_priv,
> > case PORT_B:
> > case PORT_C:
> > case PORT_D:
> > + case PORT_F:
>
>
> > return DP_AUX_CH_DATA(port, index);
> > default:
> > MISSING_CASE(port);
> > @@ -6224,6 +6229,9 @@ intel_dp_init_connector_port_info(struct
> > intel_digital_port *intel_dig_port)
> > /* FIXME: Check VBT for actual wiring of PORT E */
> > intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D;
> > break;
> > + case PORT_F:
> > + intel_dp->aux_power_domain = POWER_DOMAIN_AUX_F;
> > + break;
> > default:
> > MISSING_CASE(encoder->port);
> > }
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 5b1aa4b9c72c..27174d49a529 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -124,6 +124,8 @@ intel_display_power_domain_str(enum
> > intel_display_power_domain domain)
> > return "AUX_C";
> > case POWER_DOMAIN_AUX_D:
> > return "AUX_D";
> > + case POWER_DOMAIN_AUX_F:
> > + return "AUX_F";
> > case POWER_DOMAIN_GMBUS:
> > return "GMBUS";
> > case POWER_DOMAIN_INIT:
> > @@ -1855,6 +1857,9 @@ void intel_display_power_put(struct drm_i915_private
> > *dev_priv,
> > #define CNL_DISPLAY_AUX_D_POWER_DOMAINS ( \
> > BIT_ULL(POWER_DOMAIN_AUX_D) | \
> > BIT_ULL(POWER_DOMAIN_INIT))
> > +#define CNL_DISPLAY_AUX_F_POWER_DOMAINS ( \
> > + BIT_ULL(POWER_DOMAIN_AUX_F) | \
> > + BIT_ULL(POWER_DOMAIN_INIT))
> > #define CNL_DISPLAY_DC_OFF_POWER_DOMAINS ( \
> > CNL_DISPLAY_POWERWELL_2_POWER_DOMAINS | \
> > BIT_ULL(POWER_DOMAIN_GT_IRQ) | \
> > @@ -2405,6 +2410,12 @@ static struct i915_power_well cnl_power_wells[] = {
> > .ops = &hsw_power_well_ops,
> > .id = SKL_DISP_PW_DDI_D,
> > },
> > + {
> > + .name = "AUX F",
> > + .domains = CNL_DISPLAY_AUX_F_POWER_DOMAINS,
> > + .ops = &hsw_power_well_ops,
> > + .id = CNL_DISP_PW_AUX_F,
> > + },
>
> This breaks CNL's without port F and gets fixed in a later patch. I am
> wondering if we should set ->power_well_count -= 1 here, and set it
> properly later.
Well... it is not harmful... just a warning.... But now with
IS_CNL_WITH_PORT_F defined along with PCI IDs and having that
simple power well solution Imre proposed we can sort the patches
in a way that those comes first.
>
>
>
> > };
> >
> > static int
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
