Re: [Intel-gfx] [PATCH v4] drivers: i915: Default backlight PWM frequency

2012-01-17 Thread Daniel Vetter
On Tue, Nov 15, 2011 at 07:47:58PM +0100, Takashi Iwai wrote:
 At Fri, 11 Nov 2011 14:12:58 -0800,
 Simon Que wrote:
  
  If the firmware did not initialize the backlight PWM registers, set up a
  default PWM frequency of 200 Hz.  This is determined using the following
  formula:
  
freq = refclk / (128 * pwm_max)
  
  The PWM register allows the max PWM value to be set.  So we want to use
  the formula, where freq = 200:
  
pwm_max = refclk / (128 * freq)
  
  This patch will, in the case of missing PWM register initialization
  values, look for the reference clock frequency.  Based on that, it sets
  an appropriate max PWM value for a frequency of 200 Hz.
  
  If no refclk frequency is found, the max PWM will be zero, which results
  in no change to the PWM registers.
  
  Signed-off-by: Simon Que s...@chromium.org
  ---
   drivers/gpu/drm/i915/intel_panel.c |   38 
  ++-
   1 files changed, 32 insertions(+), 6 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/intel_panel.c 
  b/drivers/gpu/drm/i915/intel_panel.c
  index f15388c..dda5de2 100644
  --- a/drivers/gpu/drm/i915/intel_panel.c
  +++ b/drivers/gpu/drm/i915/intel_panel.c
  @@ -32,6 +32,12 @@
   
   #define PCI_LBPC 0xf4 /* legacy/combination backlight modes */
   
  +/* These are used to calculate a reasonable default when firmware has not
  + * configured a maximum PWM frequency, with 200Hz as the current default 
  target.
  + */
  +#define DEFAULT_BACKLIGHT_PWM_FREQ   200
  +#define BACKLIGHT_REFCLK_DIVISOR 128
  +
   void
   intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
 struct drm_display_mode *adjusted_mode)
  @@ -129,12 +135,32 @@ static int is_backlight_combination_mode(struct 
  drm_device *dev)
  return 0;
   }
   
  +static void i915_set_default_max_backlight(struct drm_i915_private 
  *dev_priv)
  +{
  +   u32 refclk_freq_mhz = 0;
  +   u32 max_pwm;
  +
  +   if (HAS_PCH_SPLIT(dev_priv-dev))
  +   refclk_freq_mhz = I915_READ(PCH_RAWCLK_FREQ)  RAWCLK_FREQ_MASK;
  +   else if (dev_priv-lvds_use_ssc)
  +   refclk_freq_mhz = dev_priv-lvds_ssc_freq;
  +
  +   max_pwm = refclk_freq_mhz * 100 /
  +   (BACKLIGHT_REFCLK_DIVISOR * DEFAULT_BACKLIGHT_PWM_FREQ);
  +
  +   if (HAS_PCH_SPLIT(dev_priv-dev))
  +   dev_priv-saveBLC_PWM_CTL2 = max_pwm  16;
  +   else if (IS_PINEVIEW(dev_priv-dev))
  +   dev_priv-saveBLC_PWM_CTL = max_pwm  17;
  +   else
  +   dev_priv-saveBLC_PWM_CTL = max_pwm  16;
 
 Is the pineview case really correct?
 The special handling for pineview in some places in intel_panel.c is
 just for omitting the bit 0, IIRC.  It doesn't mean that the value is
 twice larger.
 
 BTW, this handling of bit 0 seems necessary not only for pineview but
 for the older chips (gen  4) in general, too, as being discussed in
 another thread of LKML.  915GM hits the with problem of bit-0, for
 example.

Do we still need this patch? If so, can you please address Takashi's
comment, on a quick check he seems to have a point.
-Daniel
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v4] drivers: i915: Default backlight PWM frequency

2011-11-15 Thread Takashi Iwai
At Fri, 11 Nov 2011 14:12:58 -0800,
Simon Que wrote:
 
 If the firmware did not initialize the backlight PWM registers, set up a
 default PWM frequency of 200 Hz.  This is determined using the following
 formula:
 
   freq = refclk / (128 * pwm_max)
 
 The PWM register allows the max PWM value to be set.  So we want to use
 the formula, where freq = 200:
 
   pwm_max = refclk / (128 * freq)
 
 This patch will, in the case of missing PWM register initialization
 values, look for the reference clock frequency.  Based on that, it sets
 an appropriate max PWM value for a frequency of 200 Hz.
 
 If no refclk frequency is found, the max PWM will be zero, which results
 in no change to the PWM registers.
 
 Signed-off-by: Simon Que s...@chromium.org
 ---
  drivers/gpu/drm/i915/intel_panel.c |   38 ++-
  1 files changed, 32 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_panel.c 
 b/drivers/gpu/drm/i915/intel_panel.c
 index f15388c..dda5de2 100644
 --- a/drivers/gpu/drm/i915/intel_panel.c
 +++ b/drivers/gpu/drm/i915/intel_panel.c
 @@ -32,6 +32,12 @@
  
  #define PCI_LBPC 0xf4 /* legacy/combination backlight modes */
  
 +/* These are used to calculate a reasonable default when firmware has not
 + * configured a maximum PWM frequency, with 200Hz as the current default 
 target.
 + */
 +#define DEFAULT_BACKLIGHT_PWM_FREQ   200
 +#define BACKLIGHT_REFCLK_DIVISOR 128
 +
  void
  intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
  struct drm_display_mode *adjusted_mode)
 @@ -129,12 +135,32 @@ static int is_backlight_combination_mode(struct 
 drm_device *dev)
   return 0;
  }
  
 +static void i915_set_default_max_backlight(struct drm_i915_private *dev_priv)
 +{
 + u32 refclk_freq_mhz = 0;
 + u32 max_pwm;
 +
 + if (HAS_PCH_SPLIT(dev_priv-dev))
 + refclk_freq_mhz = I915_READ(PCH_RAWCLK_FREQ)  RAWCLK_FREQ_MASK;
 + else if (dev_priv-lvds_use_ssc)
 + refclk_freq_mhz = dev_priv-lvds_ssc_freq;
 +
 + max_pwm = refclk_freq_mhz * 100 /
 + (BACKLIGHT_REFCLK_DIVISOR * DEFAULT_BACKLIGHT_PWM_FREQ);
 +
 + if (HAS_PCH_SPLIT(dev_priv-dev))
 + dev_priv-saveBLC_PWM_CTL2 = max_pwm  16;
 + else if (IS_PINEVIEW(dev_priv-dev))
 + dev_priv-saveBLC_PWM_CTL = max_pwm  17;
 + else
 + dev_priv-saveBLC_PWM_CTL = max_pwm  16;

Is the pineview case really correct?
The special handling for pineview in some places in intel_panel.c is
just for omitting the bit 0, IIRC.  It doesn't mean that the value is
twice larger.

BTW, this handling of bit 0 seems necessary not only for pineview but
for the older chips (gen  4) in general, too, as being discussed in
another thread of LKML.  915GM hits the with problem of bit-0, for
example.


Takashi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v4] drivers: i915: Default backlight PWM frequency

2011-11-11 Thread Simon Que
If the firmware did not initialize the backlight PWM registers, set up a
default PWM frequency of 200 Hz.  This is determined using the following
formula:

  freq = refclk / (128 * pwm_max)

The PWM register allows the max PWM value to be set.  So we want to use
the formula, where freq = 200:

  pwm_max = refclk / (128 * freq)

This patch will, in the case of missing PWM register initialization
values, look for the reference clock frequency.  Based on that, it sets
an appropriate max PWM value for a frequency of 200 Hz.

If no refclk frequency is found, the max PWM will be zero, which results
in no change to the PWM registers.

Signed-off-by: Simon Que s...@chromium.org
---
 drivers/gpu/drm/i915/intel_panel.c |   38 ++-
 1 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_panel.c 
b/drivers/gpu/drm/i915/intel_panel.c
index f15388c..dda5de2 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -32,6 +32,12 @@
 
 #define PCI_LBPC 0xf4 /* legacy/combination backlight modes */
 
+/* These are used to calculate a reasonable default when firmware has not
+ * configured a maximum PWM frequency, with 200Hz as the current default 
target.
+ */
+#define DEFAULT_BACKLIGHT_PWM_FREQ   200
+#define BACKLIGHT_REFCLK_DIVISOR 128
+
 void
 intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
   struct drm_display_mode *adjusted_mode)
@@ -129,12 +135,32 @@ static int is_backlight_combination_mode(struct 
drm_device *dev)
return 0;
 }
 
+static void i915_set_default_max_backlight(struct drm_i915_private *dev_priv)
+{
+   u32 refclk_freq_mhz = 0;
+   u32 max_pwm;
+
+   if (HAS_PCH_SPLIT(dev_priv-dev))
+   refclk_freq_mhz = I915_READ(PCH_RAWCLK_FREQ)  RAWCLK_FREQ_MASK;
+   else if (dev_priv-lvds_use_ssc)
+   refclk_freq_mhz = dev_priv-lvds_ssc_freq;
+
+   max_pwm = refclk_freq_mhz * 100 /
+   (BACKLIGHT_REFCLK_DIVISOR * DEFAULT_BACKLIGHT_PWM_FREQ);
+
+   if (HAS_PCH_SPLIT(dev_priv-dev))
+   dev_priv-saveBLC_PWM_CTL2 = max_pwm  16;
+   else if (IS_PINEVIEW(dev_priv-dev))
+   dev_priv-saveBLC_PWM_CTL = max_pwm  17;
+   else
+   dev_priv-saveBLC_PWM_CTL = max_pwm  16;
+}
+
 static u32 i915_read_blc_pwm_ctl(struct drm_i915_private *dev_priv)
 {
u32 val;
 
-   /* Restore the CTL value if it lost, e.g. GPU reset */
-
+   /* Restore the CTL value if it was lost, e.g. GPU reset */
if (HAS_PCH_SPLIT(dev_priv-dev)) {
val = I915_READ(BLC_PWM_PCH_CTL2);
if (dev_priv-saveBLC_PWM_CTL2 == 0) {
@@ -168,11 +194,11 @@ u32 intel_panel_get_max_backlight(struct drm_device *dev)
 
max = i915_read_blc_pwm_ctl(dev_priv);
if (max == 0) {
-   /* XXX add code here to query mode clock or hardware clock
-* and program max PWM appropriately.
+   /* If backlight PWM registers have not been set, set them to
+* default backlight PWM settings.
 */
-   printk_once(KERN_WARNING fixme: max PWM is zero.\n);
-   return 1;
+   i915_set_default_max_backlight(dev_priv);
+   max = i915_read_blc_pwm_ctl(dev_priv);
}
 
if (HAS_PCH_SPLIT(dev)) {
-- 
1.7.2.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v4] drivers: i915: Default backlight PWM frequency

2011-11-11 Thread Olof Johansson
On Fri, Nov 11, 2011 at 02:12:58PM -0800, Simon Que wrote:
 If the firmware did not initialize the backlight PWM registers, set up a
 default PWM frequency of 200 Hz.  This is determined using the following
 formula:
 
   freq = refclk / (128 * pwm_max)
 
 The PWM register allows the max PWM value to be set.  So we want to use
 the formula, where freq = 200:
 
   pwm_max = refclk / (128 * freq)
 
 This patch will, in the case of missing PWM register initialization
 values, look for the reference clock frequency.  Based on that, it sets
 an appropriate max PWM value for a frequency of 200 Hz.
 
 If no refclk frequency is found, the max PWM will be zero, which results
 in no change to the PWM registers.
 
 Signed-off-by: Simon Que s...@chromium.org

Acked-by: Olof Johansson o...@lixom.net

Looks much better. I'm OK with this solution. Matthew?


-Olof



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v4] drivers: i915: Default backlight PWM frequency

2011-11-11 Thread Matthew Garrett
On Fri, Nov 11, 2011 at 02:17:20PM -0800, Olof Johansson wrote:
 On Fri, Nov 11, 2011 at 02:12:58PM -0800, Simon Que wrote:
  If the firmware did not initialize the backlight PWM registers, set up a
  default PWM frequency of 200 Hz.  This is determined using the following
  formula:
  
freq = refclk / (128 * pwm_max)
  
  The PWM register allows the max PWM value to be set.  So we want to use
  the formula, where freq = 200:
  
pwm_max = refclk / (128 * freq)
  
  This patch will, in the case of missing PWM register initialization
  values, look for the reference clock frequency.  Based on that, it sets
  an appropriate max PWM value for a frequency of 200 Hz.
  
  If no refclk frequency is found, the max PWM will be zero, which results
  in no change to the PWM registers.
  
  Signed-off-by: Simon Que s...@chromium.org
 
 Acked-by: Olof Johansson o...@lixom.net
 
 Looks much better. I'm OK with this solution. Matthew?

I'd still prefer this to come from the firmware in some way, but in the 
absence of the awesome let's go with the good.

Acked-by: Matthew Garrett m...@redhat.com

-- 
Matthew Garrett | mj...@srcf.ucam.org
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx