| Message ID | 20181127003710.18618-4-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/9] drm/i915: Disable PSR in Apple panels | expand |
On Mon, Nov 26, 2018 at 04:37:05PM -0800, José Roberto de Souza wrote: > For ICL the bit 12 of CHICKEN_TRANS is reserved so we should not > touch it and as by default VSC_DATA_SEL_SOFTWARE_CONTROL is already > unset in gen10 + GLK we can just drop it and fix for both gens. > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> Ok, this patch seems right according to spec. Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> But I wonder now if we need intel_psr_setup_vsc() at all for platforms different than gen9. Because description of this bit is: This field enables the programmable header for the PSR2 VSC packet. Without the programmable version I would assume display engine is now responsible for setting header entirely? Art? > --- > drivers/gpu/drm/i915/intel_psr.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c > index 607c3ec41679..7607a58a6ec0 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -635,17 +635,14 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > hsw_psr_setup_aux(intel_dp); > > - if (dev_priv->psr.psr2_enabled) { > + if (dev_priv->psr.psr2_enabled && (IS_GEN9(dev_priv) && > + !IS_GEMINILAKE(dev_priv))) { > i915_reg_t reg = gen9_chicken_trans_reg(dev_priv, > cpu_transcoder); > u32 chicken = I915_READ(reg); > > - if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv)) > - chicken |= (PSR2_VSC_ENABLE_PROG_HEADER > - | PSR2_ADD_VERTICAL_LINE_COUNT); > - > - else > - chicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL; > + chicken |= PSR2_VSC_ENABLE_PROG_HEADER | > + PSR2_ADD_VERTICAL_LINE_COUNT; > I915_WRITE(reg, chicken); > } > > -- > 2.19.2 >
On Thu, 2018-11-29 at 14:15 -0800, Rodrigo Vivi wrote: > On Mon, Nov 26, 2018 at 04:37:05PM -0800, José Roberto de Souza > wrote: > > For ICL the bit 12 of CHICKEN_TRANS is reserved so we should not > > touch it and as by default VSC_DATA_SEL_SOFTWARE_CONTROL is already > > unset in gen10 + GLK we can just drop it and fix for both gens. > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > Ok, this patch seems right according to spec. > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > But I wonder now if we need intel_psr_setup_vsc() at all for > platforms different than gen9. > > Because description of this bit is: > This field enables the programmable header for the PSR2 VSC packet. > > Without the programmable version I would assume display > engine is now responsible for setting header entirely? As this was in a chicken register in gen 9 my guess is that it was fixed on newer gens and as it is required for PSR2 we don't need to manually enable it in driver but Art could confirm it. > > Art? > > > --- > > drivers/gpu/drm/i915/intel_psr.c | 11 ++++------- > > 1 file changed, 4 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > > b/drivers/gpu/drm/i915/intel_psr.c > > index 607c3ec41679..7607a58a6ec0 100644 > > --- a/drivers/gpu/drm/i915/intel_psr.c > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > @@ -635,17 +635,14 @@ static void intel_psr_enable_source(struct > > intel_dp *intel_dp, > > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > > hsw_psr_setup_aux(intel_dp); > > > > - if (dev_priv->psr.psr2_enabled) { > > + if (dev_priv->psr.psr2_enabled && (IS_GEN9(dev_priv) && > > + !IS_GEMINILAKE(dev_priv))) { > > i915_reg_t reg = gen9_chicken_trans_reg(dev_priv, > > cpu_transcoder) > > ; > > u32 chicken = I915_READ(reg); > > > > - if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv)) > > - chicken |= (PSR2_VSC_ENABLE_PROG_HEADER > > - | PSR2_ADD_VERTICAL_LINE_COUNT); > > - > > - else > > - chicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL; > > + chicken |= PSR2_VSC_ENABLE_PROG_HEADER | > > + PSR2_ADD_VERTICAL_LINE_COUNT; > > I915_WRITE(reg, chicken); > > } > > > > -- > > 2.19.2 > >
I'll check on it. > -----Original Message----- > From: Souza, Jose > Sent: Thursday, 29 November, 2018 3:47 PM > To: Vivi, Rodrigo <rodrigo.vivi@intel.com> > Cc: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; > Runyan, Arthur J <arthur.j.runyan@intel.com>; Pandiyan, Dhinakaran > <dhinakaran.pandiyan@intel.com> > Subject: Re: [PATCH 4/9] drm/i915/icl: Do not change reserved registers > related to PSR2 > > On Thu, 2018-11-29 at 14:15 -0800, Rodrigo Vivi wrote: > > On Mon, Nov 26, 2018 at 04:37:05PM -0800, José Roberto de Souza > > wrote: > > > For ICL the bit 12 of CHICKEN_TRANS is reserved so we should not > > > touch it and as by default VSC_DATA_SEL_SOFTWARE_CONTROL is > already > > > unset in gen10 + GLK we can just drop it and fix for both gens. > > > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > > Ok, this patch seems right according to spec. > > > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > > But I wonder now if we need intel_psr_setup_vsc() at all for > > platforms different than gen9. > > > > Because description of this bit is: > > This field enables the programmable header for the PSR2 VSC packet. > > > > Without the programmable version I would assume display > > engine is now responsible for setting header entirely? > > As this was in a chicken register in gen 9 my guess is that it was > fixed on newer gens and as it is required for PSR2 we don't need to > manually enable it in driver but Art could confirm it. > > > > > Art? > > > > > --- > > > drivers/gpu/drm/i915/intel_psr.c | 11 ++++------- > > > 1 file changed, 4 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > > > b/drivers/gpu/drm/i915/intel_psr.c > > > index 607c3ec41679..7607a58a6ec0 100644 > > > --- a/drivers/gpu/drm/i915/intel_psr.c > > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > > @@ -635,17 +635,14 @@ static void intel_psr_enable_source(struct > > > intel_dp *intel_dp, > > > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > > > hsw_psr_setup_aux(intel_dp); > > > > > > - if (dev_priv->psr.psr2_enabled) { > > > + if (dev_priv->psr.psr2_enabled && (IS_GEN9(dev_priv) && > > > + !IS_GEMINILAKE(dev_priv))) { > > > i915_reg_t reg = gen9_chicken_trans_reg(dev_priv, > > > cpu_transcoder) > > > ; > > > u32 chicken = I915_READ(reg); > > > > > > - if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv)) > > > - chicken |= (PSR2_VSC_ENABLE_PROG_HEADER > > > - | PSR2_ADD_VERTICAL_LINE_COUNT); > > > - > > > - else > > > - chicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL; > > > + chicken |= PSR2_VSC_ENABLE_PROG_HEADER | > > > + PSR2_ADD_VERTICAL_LINE_COUNT; > > > I915_WRITE(reg, chicken); > > > } > > > > > > -- > > > 2.19.2 > > >
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 607c3ec41679..7607a58a6ec0 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -635,17 +635,14 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) hsw_psr_setup_aux(intel_dp); - if (dev_priv->psr.psr2_enabled) { + if (dev_priv->psr.psr2_enabled && (IS_GEN9(dev_priv) && + !IS_GEMINILAKE(dev_priv))) { i915_reg_t reg = gen9_chicken_trans_reg(dev_priv, cpu_transcoder); u32 chicken = I915_READ(reg); - if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv)) - chicken |= (PSR2_VSC_ENABLE_PROG_HEADER - | PSR2_ADD_VERTICAL_LINE_COUNT); - - else - chicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL; + chicken |= PSR2_VSC_ENABLE_PROG_HEADER | + PSR2_ADD_VERTICAL_LINE_COUNT; I915_WRITE(reg, chicken); }
For ICL the bit 12 of CHICKEN_TRANS is reserved so we should not touch it and as by default VSC_DATA_SEL_SOFTWARE_CONTROL is already unset in gen10 + GLK we can just drop it and fix for both gens. Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/intel_psr.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)