On Mon, Jun 13, 2022 at 7:08 PM Sam Ravnborg sam@ravnborg.org wrote:
Hi Patrick.
On Mon, Jun 13, 2022 at 02:34:18PM +0200, Patrik Jakobsson wrote:
These functions mostly do the same thing so unify them into one. All unified lvds code will live in gma_lvds.c.
Signed-off-by: Patrik Jakobsson patrik.r.jakobsson@gmail.com
+/*
- Returns the maximum level of the backlight duty cycle field.
- */
I find this function quite un-readable.
+u32 gma_lvds_get_max_backlight(struct drm_device *dev) +{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);u32 retval;if (gma_power_begin(dev, false)) {retval = ((REG_READ(BLC_PWM_CTL) &BACKLIGHT_MODULATION_FREQ_MASK) >>BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;gma_power_end(dev);} elseretval = ((dev_priv->regs.saveBLC_PWM_CTL &BACKLIGHT_MODULATION_FREQ_MASK) >>BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;return retval;+}
Maybe like this - which should be the same functionality:
u32 gma_lvds_get_max_backlight(struct drm_device *dev) { struct drm_psb_private *dev_priv = to_drm_psb_private(dev); u32 pwmctl;
if (gma_power_begin(dev, false)) { pwmctl = REG_READ(BLC_PWM_CTL); gma_power_end(dev); } else { pwmctl = dev_priv->regs.saveBLC_PWM_CTL; } pwmctl = pwmctl & BACKLIGHT_MODULATION_FREQ_MASK; return (pwmctl >> BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;}
this is more or less the same as in psb_intel_lvds_get_max_backlight().
With or without this change the patch is: Reviewed-by: Sam Ravnborg sam@ravnborg.org
Hi Sam, Thanks for having a look.
I've intentionally tried to change as little as possible from the version I copied so that any functional change is easy to spot and the series becomes easy to review. Would it be ok if I do cleanups as a followup series?
-Patrik
diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h new file mode 100644 index 000000000000..2a9ce8ee3fa7 --- /dev/null +++ b/drivers/gpu/drm/gma500/gma_lvds.h @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: GPL-2.0-only */
+/*
- Copyright © 2006-2011 Intel Corporation
- */
+#ifndef _GMA_LVDS_H +#define _GMA_LVDS_H
+u32 gma_lvds_get_max_backlight(struct drm_device *dev);
+#endif diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c index 9c9ebf8e29c4..4913baca7ae2 100644 --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c @@ -20,6 +20,7 @@ #include "psb_drv.h" #include "psb_intel_drv.h" #include "psb_intel_reg.h" +#include "gma_lvds.h"
/* The max/min PWM frequency in BPCR[31:17] - */ /* The smallest number is 1 (not 0) that can fit in the @@ -170,25 +171,6 @@ static void oaktrail_lvds_prepare(struct drm_encoder *encoder) gma_power_end(dev); }
-static u32 oaktrail_lvds_get_max_backlight(struct drm_device *dev) -{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);u32 ret;if (gma_power_begin(dev, false)) {ret = ((REG_READ(BLC_PWM_CTL) &BACKLIGHT_MODULATION_FREQ_MASK) >>BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;gma_power_end(dev);} elseret = ((dev_priv->regs.saveBLC_PWM_CTL &BACKLIGHT_MODULATION_FREQ_MASK) >>BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;return ret;-}
static void oaktrail_lvds_commit(struct drm_encoder *encoder) { struct drm_device *dev = encoder->dev; @@ -197,8 +179,7 @@ static void oaktrail_lvds_commit(struct drm_encoder *encoder) struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
if (mode_dev->backlight_duty_cycle == 0)
mode_dev->backlight_duty_cycle =oaktrail_lvds_get_max_backlight(dev);
mode_dev->backlight_duty_cycle = gma_lvds_get_max_backlight(dev); oaktrail_lvds_set_power(dev, gma_encoder, true);}
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c index 7ee6c8ce103b..371c202a15ce 100644 --- a/drivers/gpu/drm/gma500/psb_intel_lvds.c +++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c @@ -18,6 +18,7 @@ #include "psb_drv.h" #include "psb_intel_drv.h" #include "psb_intel_reg.h" +#include "gma_lvds.h"
/*
- LVDS I2C backlight control macros
@@ -52,32 +53,6 @@ struct psb_intel_lvds_priv { struct gma_i2c_chan *i2c_bus; };
-/*
- Returns the maximum level of the backlight duty cycle field.
- */
-static u32 psb_intel_lvds_get_max_backlight(struct drm_device *dev) -{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);u32 ret;if (gma_power_begin(dev, false)) {ret = REG_READ(BLC_PWM_CTL);gma_power_end(dev);} else /* Powered off, use the saved value */ret = dev_priv->regs.saveBLC_PWM_CTL;/* Top 15bits hold the frequency mask */ret = (ret & BACKLIGHT_MODULATION_FREQ_MASK) >>BACKLIGHT_MODULATION_FREQ_SHIFT;ret *= 2; /* Return a 16bit range as needed for setting */if (ret == 0)dev_err(dev->dev, "BL bug: Reg %08x save %08X\n",REG_READ(BLC_PWM_CTL), dev_priv->regs.saveBLC_PWM_CTL);return ret;-}
/*
- Set LVDS backlight level by I2C command
@@ -131,7 +106,7 @@ static int psb_lvds_pwm_set_brightness(struct drm_device *dev, int level) u32 max_pwm_blc; u32 blc_pwm_duty_cycle;
max_pwm_blc = psb_intel_lvds_get_max_backlight(dev);
max_pwm_blc = gma_lvds_get_max_backlight(dev); /*BLC_PWM_CTL Should be initiated while backlight device init*/ BUG_ON(max_pwm_blc == 0);@@ -176,7 +151,7 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level) /*
- Sets the backlight level.
- level: backlight level, from 0 to psb_intel_lvds_get_max_backlight().
*/
- level: backlight level, from 0 to gma_lvds_get_max_backlight().
static void psb_intel_lvds_set_backlight(struct drm_device *dev, int level) { @@ -275,8 +250,7 @@ static void psb_intel_lvds_save(struct drm_connector *connector) * just make it full brightness */ if (dev_priv->backlight_duty_cycle == 0)
dev_priv->backlight_duty_cycle =psb_intel_lvds_get_max_backlight(dev);
dev_priv->backlight_duty_cycle = gma_lvds_get_max_backlight(dev); dev_dbg(dev->dev, "(0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x)\n", lvds_priv->savePP_ON,@@ -445,7 +419,7 @@ static void psb_intel_lvds_commit(struct drm_encoder *encoder)
if (mode_dev->backlight_duty_cycle == 0) mode_dev->backlight_duty_cycle =
psb_intel_lvds_get_max_backlight(dev);
gma_lvds_get_max_backlight(dev); psb_intel_lvds_set_power(dev, true);}
2.36.1