| Message ID | 1380317977-1009-2-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, 28 Sep 2013, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > With the connector and pipe passed around, we can now set the backlight > on the right pipe on VLV/BYT. > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > --- > drivers/gpu/drm/i915/i915_reg.h | 10 ++++++ > drivers/gpu/drm/i915/intel_panel.c | 65 +++++++++++++++++++++++++++++++------- > 2 files changed, 63 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 96fd2ce..a3b9508 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -2295,6 +2295,16 @@ > > #define PFIT_AUTO_RATIOS (dev_priv->info->display_mmio_offset + 0x61238) > > +#define _VLV_BLC_PWM_CTL2_A (dev_priv->info->display_mmio_offset + 0x61250) > +#define _VLV_BLC_PWM_CTL2_B (dev_priv->info->display_mmio_offset + 0x61350) > +#define VLV_BLC_PWM_CTL2(pipe) _PIPE(pipe, _VLV_BLC_PWM_CTL2_A, \ > + _VLV_BLC_PWM_CTL2_B) > + > +#define _VLV_BLC_PWM_CTL_A (dev_priv->info->display_mmio_offset + 0x61254) > +#define _VLV_BLC_PWM_CTL_B (dev_priv->info->display_mmio_offset + 0x61354) > +#define VLV_BLC_PWM_CTL(pipe) _PIPE(pipe, _VLV_BLC_PWM_CTL_A, \ > + _VLV_BLC_PWM_CTL_B) > + > /* Backlight control */ > #define BLC_PWM_CTL2 (dev_priv->info->display_mmio_offset + 0x61250) /* 965+ only */ > #define BLM_PWM_ENABLE (1 << 31) > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > index 5122f58..03f8450 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -329,6 +329,9 @@ static int is_backlight_combination_mode(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > > + if (IS_VALLEYVIEW(dev)) > + return 0; > + Superfluous since commit d17d711d3e34d71dada5da5b508738197d749112 Author: Jani Nikula <jani.nikula@intel.com> Date: Tue Sep 24 16:44:39 2013 +0300 drm/i915: backlight combination mode bit is gen4 only > if (IS_GEN4(dev)) > return I915_READ(BLC_PWM_CTL2) & BLM_COMBINATION_MODE; > > @@ -358,6 +361,21 @@ static u32 i915_read_blc_pwm_ctl(struct drm_device *dev, enum pipe pipe) > val = dev_priv->regfile.saveBLC_PWM_CTL2; > I915_WRITE(BLC_PWM_PCH_CTL2, val); > } > + } else if (IS_VALLEYVIEW(dev)) { > + val = I915_READ(VLV_BLC_PWM_CTL(pipe)); > + if (dev_priv->regfile.saveBLC_PWM_CTL == 0) { > + dev_priv->regfile.saveBLC_PWM_CTL = val; > + dev_priv->regfile.saveBLC_PWM_CTL2 = > + I915_READ(VLV_BLC_PWM_CTL2(pipe)); > + } else if (val == 0) { > + val = dev_priv->regfile.saveBLC_PWM_CTL; > + I915_WRITE(VLV_BLC_PWM_CTL(pipe), val); > + I915_WRITE(VLV_BLC_PWM_CTL2(pipe), > + dev_priv->regfile.saveBLC_PWM_CTL2); > + } > + > + if (!val) > + val = 0x0f42ffff; I really really wish I knew what this stuff in this function is (was) really needed for. commit 0b0b053a3949f5c467c3b3ba135d4c161f9fbd00 Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Tue Nov 23 09:45:50 2010 +0000 drm/i915/panel: Restore saved value of BLC_PWM_CTL After a GPU reset, the backlight controller registers may be also reset to 0. In that case we should restore those to the original values programmed by the BIOS. Note that we still lack the code to handle the case where the BIOS failed to program those registers at all... ...if backlight controller registers _may_ be reset to 0, what else can? Anyway, I think you need to add per pipe save/restore to i915_save_display/i915_restore_display too. > } else { > val = I915_READ(BLC_PWM_CTL); > if (dev_priv->regfile.saveBLC_PWM_CTL == 0) { > @@ -372,9 +390,6 @@ static u32 i915_read_blc_pwm_ctl(struct drm_device *dev, enum pipe pipe) > I915_WRITE(BLC_PWM_CTL2, > dev_priv->regfile.saveBLC_PWM_CTL2); > } > - > - if (IS_VALLEYVIEW(dev) && !val) > - val = 0x0f42ffff; > } > > return val; > @@ -435,13 +450,19 @@ static u32 intel_panel_get_backlight(struct drm_device *dev, > struct drm_i915_private *dev_priv = dev->dev_private; > u32 val; > unsigned long flags; > + int reg; > > spin_lock_irqsave(&dev_priv->backlight.lock, flags); > > if (HAS_PCH_SPLIT(dev)) { > val = I915_READ(BLC_PWM_CPU_CTL) & BACKLIGHT_DUTY_CYCLE_MASK; > } else { > - val = I915_READ(BLC_PWM_CTL) & BACKLIGHT_DUTY_CYCLE_MASK; > + if (IS_VALLEYVIEW(dev)) > + reg = VLV_BLC_PWM_CTL(pipe); > + else > + reg = BLC_PWM_CTL; > + > + val = I915_READ(reg) & BACKLIGHT_DUTY_CYCLE_MASK; > if (INTEL_INFO(dev)->gen < 4) > val >>= 1; > > @@ -473,6 +494,7 @@ static void intel_panel_actually_set_backlight(struct drm_device *dev, > { > struct drm_i915_private *dev_priv = dev->dev_private; > u32 tmp; > + int reg; > > DRM_DEBUG_DRIVER("set backlight PWM = %d\n", level); > level = intel_panel_compute_brightness(dev, pipe, level); > @@ -493,11 +515,16 @@ static void intel_panel_actually_set_backlight(struct drm_device *dev, > pci_write_config_byte(dev->pdev, PCI_LBPC, lbpc); > } > > - tmp = I915_READ(BLC_PWM_CTL); > + if (IS_VALLEYVIEW(dev)) > + reg = VLV_BLC_PWM_CTL(pipe); > + else > + reg = BLC_PWM_CTL; > + > + tmp = I915_READ(reg); > if (INTEL_INFO(dev)->gen < 4) > level <<= 1; > tmp &= ~BACKLIGHT_DUTY_CYCLE_MASK; > - I915_WRITE(BLC_PWM_CTL, tmp | level); > + I915_WRITE(reg, tmp | level); > } > > /* set backlight brightness to level in range [0..max] */ > @@ -560,7 +587,12 @@ void intel_panel_disable_backlight(struct intel_connector *connector) > if (INTEL_INFO(dev)->gen >= 4) { > uint32_t reg, tmp; > > - reg = HAS_PCH_SPLIT(dev) ? BLC_PWM_CPU_CTL2 : BLC_PWM_CTL2; > + if (HAS_PCH_SPLIT(dev)) > + reg = BLC_PWM_CPU_CTL2; > + else if (IS_VALLEYVIEW(dev)) > + reg = VLV_BLC_PWM_CTL2(pipe); > + else > + reg = BLC_PWM_CTL2; > > I915_WRITE(reg, I915_READ(reg) & ~BLM_PWM_ENABLE); > > @@ -596,8 +628,12 @@ void intel_panel_enable_backlight(struct intel_connector *connector) > if (INTEL_INFO(dev)->gen >= 4) { > uint32_t reg, tmp; > > - reg = HAS_PCH_SPLIT(dev) ? BLC_PWM_CPU_CTL2 : BLC_PWM_CTL2; > - > + if (HAS_PCH_SPLIT(dev)) > + reg = BLC_PWM_CPU_CTL2; > + else if (IS_VALLEYVIEW(dev)) > + reg = VLV_BLC_PWM_CTL2(pipe); > + else > + reg = BLC_PWM_CTL2; > > tmp = I915_READ(reg); > > @@ -649,9 +685,14 @@ static void intel_panel_init_backlight_regs(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > > if (IS_VALLEYVIEW(dev)) { > - u32 cur_val = I915_READ(BLC_PWM_CTL) & > - BACKLIGHT_DUTY_CYCLE_MASK; > - I915_WRITE(BLC_PWM_CTL, (0xf42 << 16) | cur_val); > + enum pipe pipe; > + > + for_each_pipe(pipe) { > + u32 cur_val = I915_READ(VLV_BLC_PWM_CTL(pipe)) & > + BACKLIGHT_DUTY_CYCLE_MASK; > + I915_WRITE(VLV_BLC_PWM_CTL(pipe), (0xf42 << 16) | > + cur_val); > + } As follow-up, we should probably check if the freq is there, and only write the magic value if it isn't. BR, Jani. > } > } > > -- > 1.8.3.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Sep 30, 2013 at 10:33 AM, Jani Nikula <jani.nikula@linux.intel.com> wrote: > Anyway, I think you need to add per pipe save/restore to > i915_save_display/i915_restore_display too. I'm trying (very slowly at that) to move the register save/restore stuff out of there into more suitable places. Either we should now what should go in there (like all the w/a bits we set in e.g. init_clock_gating) or we need to have an appropriate place to store it more localized to all the other relevant code/data. People simply forget about save/restore_display when enabling new platforms or fixing up stuff since it's somewhere completely else. -Daniel
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 96fd2ce..a3b9508 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2295,6 +2295,16 @@ #define PFIT_AUTO_RATIOS (dev_priv->info->display_mmio_offset + 0x61238) +#define _VLV_BLC_PWM_CTL2_A (dev_priv->info->display_mmio_offset + 0x61250) +#define _VLV_BLC_PWM_CTL2_B (dev_priv->info->display_mmio_offset + 0x61350) +#define VLV_BLC_PWM_CTL2(pipe) _PIPE(pipe, _VLV_BLC_PWM_CTL2_A, \ + _VLV_BLC_PWM_CTL2_B) + +#define _VLV_BLC_PWM_CTL_A (dev_priv->info->display_mmio_offset + 0x61254) +#define _VLV_BLC_PWM_CTL_B (dev_priv->info->display_mmio_offset + 0x61354) +#define VLV_BLC_PWM_CTL(pipe) _PIPE(pipe, _VLV_BLC_PWM_CTL_A, \ + _VLV_BLC_PWM_CTL_B) + /* Backlight control */ #define BLC_PWM_CTL2 (dev_priv->info->display_mmio_offset + 0x61250) /* 965+ only */ #define BLM_PWM_ENABLE (1 << 31) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 5122f58..03f8450 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -329,6 +329,9 @@ static int is_backlight_combination_mode(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; + if (IS_VALLEYVIEW(dev)) + return 0; + if (IS_GEN4(dev)) return I915_READ(BLC_PWM_CTL2) & BLM_COMBINATION_MODE; @@ -358,6 +361,21 @@ static u32 i915_read_blc_pwm_ctl(struct drm_device *dev, enum pipe pipe) val = dev_priv->regfile.saveBLC_PWM_CTL2; I915_WRITE(BLC_PWM_PCH_CTL2, val); } + } else if (IS_VALLEYVIEW(dev)) { + val = I915_READ(VLV_BLC_PWM_CTL(pipe)); + if (dev_priv->regfile.saveBLC_PWM_CTL == 0) { + dev_priv->regfile.saveBLC_PWM_CTL = val; + dev_priv->regfile.saveBLC_PWM_CTL2 = + I915_READ(VLV_BLC_PWM_CTL2(pipe)); + } else if (val == 0) { + val = dev_priv->regfile.saveBLC_PWM_CTL; + I915_WRITE(VLV_BLC_PWM_CTL(pipe), val); + I915_WRITE(VLV_BLC_PWM_CTL2(pipe), + dev_priv->regfile.saveBLC_PWM_CTL2); + } + + if (!val) + val = 0x0f42ffff; } else { val = I915_READ(BLC_PWM_CTL); if (dev_priv->regfile.saveBLC_PWM_CTL == 0) { @@ -372,9 +390,6 @@ static u32 i915_read_blc_pwm_ctl(struct drm_device *dev, enum pipe pipe) I915_WRITE(BLC_PWM_CTL2, dev_priv->regfile.saveBLC_PWM_CTL2); } - - if (IS_VALLEYVIEW(dev) && !val) - val = 0x0f42ffff; } return val; @@ -435,13 +450,19 @@ static u32 intel_panel_get_backlight(struct drm_device *dev, struct drm_i915_private *dev_priv = dev->dev_private; u32 val; unsigned long flags; + int reg; spin_lock_irqsave(&dev_priv->backlight.lock, flags); if (HAS_PCH_SPLIT(dev)) { val = I915_READ(BLC_PWM_CPU_CTL) & BACKLIGHT_DUTY_CYCLE_MASK; } else { - val = I915_READ(BLC_PWM_CTL) & BACKLIGHT_DUTY_CYCLE_MASK; + if (IS_VALLEYVIEW(dev)) + reg = VLV_BLC_PWM_CTL(pipe); + else + reg = BLC_PWM_CTL; + + val = I915_READ(reg) & BACKLIGHT_DUTY_CYCLE_MASK; if (INTEL_INFO(dev)->gen < 4) val >>= 1; @@ -473,6 +494,7 @@ static void intel_panel_actually_set_backlight(struct drm_device *dev, { struct drm_i915_private *dev_priv = dev->dev_private; u32 tmp; + int reg; DRM_DEBUG_DRIVER("set backlight PWM = %d\n", level); level = intel_panel_compute_brightness(dev, pipe, level); @@ -493,11 +515,16 @@ static void intel_panel_actually_set_backlight(struct drm_device *dev, pci_write_config_byte(dev->pdev, PCI_LBPC, lbpc); } - tmp = I915_READ(BLC_PWM_CTL); + if (IS_VALLEYVIEW(dev)) + reg = VLV_BLC_PWM_CTL(pipe); + else + reg = BLC_PWM_CTL; + + tmp = I915_READ(reg); if (INTEL_INFO(dev)->gen < 4) level <<= 1; tmp &= ~BACKLIGHT_DUTY_CYCLE_MASK; - I915_WRITE(BLC_PWM_CTL, tmp | level); + I915_WRITE(reg, tmp | level); } /* set backlight brightness to level in range [0..max] */ @@ -560,7 +587,12 @@ void intel_panel_disable_backlight(struct intel_connector *connector) if (INTEL_INFO(dev)->gen >= 4) { uint32_t reg, tmp; - reg = HAS_PCH_SPLIT(dev) ? BLC_PWM_CPU_CTL2 : BLC_PWM_CTL2; + if (HAS_PCH_SPLIT(dev)) + reg = BLC_PWM_CPU_CTL2; + else if (IS_VALLEYVIEW(dev)) + reg = VLV_BLC_PWM_CTL2(pipe); + else + reg = BLC_PWM_CTL2; I915_WRITE(reg, I915_READ(reg) & ~BLM_PWM_ENABLE); @@ -596,8 +628,12 @@ void intel_panel_enable_backlight(struct intel_connector *connector) if (INTEL_INFO(dev)->gen >= 4) { uint32_t reg, tmp; - reg = HAS_PCH_SPLIT(dev) ? BLC_PWM_CPU_CTL2 : BLC_PWM_CTL2; - + if (HAS_PCH_SPLIT(dev)) + reg = BLC_PWM_CPU_CTL2; + else if (IS_VALLEYVIEW(dev)) + reg = VLV_BLC_PWM_CTL2(pipe); + else + reg = BLC_PWM_CTL2; tmp = I915_READ(reg); @@ -649,9 +685,14 @@ static void intel_panel_init_backlight_regs(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; if (IS_VALLEYVIEW(dev)) { - u32 cur_val = I915_READ(BLC_PWM_CTL) & - BACKLIGHT_DUTY_CYCLE_MASK; - I915_WRITE(BLC_PWM_CTL, (0xf42 << 16) | cur_val); + enum pipe pipe; + + for_each_pipe(pipe) { + u32 cur_val = I915_READ(VLV_BLC_PWM_CTL(pipe)) & + BACKLIGHT_DUTY_CYCLE_MASK; + I915_WRITE(VLV_BLC_PWM_CTL(pipe), (0xf42 << 16) | + cur_val); + } } }
With the connector and pipe passed around, we can now set the backlight on the right pipe on VLV/BYT. Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/i915_reg.h | 10 ++++++ drivers/gpu/drm/i915/intel_panel.c | 65 +++++++++++++++++++++++++++++++------- 2 files changed, 63 insertions(+), 12 deletions(-)