[Nouveau] [Patch] Add a way to list the available voltages

2010-11-22 Thread Martin Peres 

Hi,

This patch makes it easy for people to set their custom_voltage file. 
Previously, they had to guess what was the available voltages.


This patch depends on my previous patch to add a custom perflvl.

Martin

From e9c880e0882bb609a9ce4ec0bf28ef99ea78714a Mon Sep 17 00:00:00 2001
From: Martin Peres mu...@mupuf.org
Date: Mon, 22 Nov 2010 09:59:16 +0100
Subject: [PATCH] Add a way to list the available voltages through sysfs

Signed-off-by: Martin Peres martin.pe...@ensi-bourges.fr
---
 drivers/gpu/drm/nouveau/nouveau_pm.c |   32 
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_pm.c b/drivers/gpu/drm/nouveau/nouveau_pm.c
index 49d8a17..b0d0691 100644
--- a/drivers/gpu/drm/nouveau/nouveau_pm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_pm.c
@@ -412,6 +412,33 @@ static DEVICE_ATTR(custom_voltage, S_IRUGO | S_IWUSR,
nouveau_pm_get_custom_voltage,
nouveau_pm_set_custom_voltage);
 
+static ssize_t
+nouveau_pm_get_voltages(struct device *d, struct device_attribute *a, char *buf)
+{
+	struct drm_device *dev = pci_get_drvdata(to_pci_dev(d));
+	struct drm_nouveau_private *dev_priv = dev-dev_private;
+	struct nouveau_pm_engine *pm = dev_priv-engine.pm;
+	struct nouveau_pm_voltage *volt = pm-voltage;
+	int cur_voltage = nouveau_voltage_gpio_get(dev);
+	char *ptr = buf;
+	int len = PAGE_SIZE;
+	int i;
+
+	for (i = 0; i  volt-nr_level; i++) {
+		int size = snprintf(ptr, len, %c %d [*10mV]\n,
+		cur_voltage == volt-level[i].voltage ? '*' : ' ',
+		volt-level[i].voltage
+		   );
+
+		ptr += size;
+		len -= size;
+	}
+
+	return strlen(buf);
+}
+
+static DEVICE_ATTR(voltages, S_IRUGO, nouveau_pm_get_voltages, NULL);
+
 static int
 nouveau_sysfs_init(struct drm_device *dev)
 {
@@ -463,6 +490,10 @@ nouveau_sysfs_init(struct drm_device *dev)
 	if (ret)
 		return ret;
 
+	ret = device_create_file(d, dev_attr_voltages);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
@@ -488,6 +519,7 @@ nouveau_sysfs_fini(struct drm_device *dev)
 	device_remove_file(d, dev_attr_custom_shader);
 	device_remove_file(d, dev_attr_custom_unk05);
 	device_remove_file(d, dev_attr_custom_voltage);
+	device_remove_file(d, dev_attr_voltages);
 }
 
 #ifdef CONFIG_HWMON
-- 
1.7.3.2


___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH 1/5] Backlight: Add backlight type

2010-11-22 Thread Matthew Garrett 
On Mon, Nov 22, 2010 at 10:17:00AM +, Richard Purdie wrote:

 I also want to make sure you think this patch is going to scale with
 multiple GPU output machines? Thats the main reason I've held off any
 patch like this as it doesn't help solve that problem as far as I can
 tell. Yes, we have the device parent information and I see later in the
 patch series you ensure the backlight is registered against the
 connector which is good. If you have an ACPI firmware control that you
 say should always be preferred, how do we know which connector device
 that corresponds to in the multiple output case? From that point of view
 this model falls apart?

The ACPI device will point at the correct PCI device. Associating it 
with the appropriate connector is theoretically possible in the case of 
open drivers, but I hadn't seen it as a high priority since (in 
practice) there's no situations where an ACPI interface will be able to 
control more than one backlight.

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

Re: [Nouveau] [PATCH 1/5] Backlight: Add backlight type

2010-11-22 Thread Matthew Garrett 
On Mon, Nov 22, 2010 at 01:35:48PM +, Richard Purdie wrote:

 Its the reverse situation I worry about. Are there situations where
 there are multiple connectors on the PCI device and the ACPI interface
 just controls one of them but controls for the other connectors may
 exist?

Yes, that's certainly possible and it'd be desirable to fix this up, but 
the same problem also applies to platform interfaces and it's typically 
unfixable there. My userspace implementation looks at the connector type 
to determine the best approach - if it's not LVDS or eDP it ignores the 
firmware and platform interfaces, so you'll fall back to the raw 
interface if it can provide support for your connector (presumably via 
ddcci, although we don't have this implemented yet)

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

[Nouveau] [PATCH 3/5] radeon: Expose backlight class device for legacy LVDS encoder

2010-11-22 Thread Matthew Garrett 
From: Michel Dänzer mic...@daenzer.net

Allows e.g. power management daemons to control the backlight level. Inspired
by the corresponding code in radeonfb.

(Updated to add backlight type and make the connector the parent device - mjg)

Signed-off-by: Michel Dänzer daen...@vmware.com
Signed-off-by: Matthew Garrett m...@redhat.com
Cc: dri-de...@lists.freedesktop.org
---
 drivers/gpu/drm/radeon/Kconfig  |1 +
 drivers/gpu/drm/radeon/radeon_connectors.c  |   15 ++
 drivers/gpu/drm/radeon/radeon_legacy_encoders.c |  257 ++-
 drivers/gpu/drm/radeon/radeon_mode.h|   10 +
 4 files changed, 277 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/radeon/Kconfig b/drivers/gpu/drm/radeon/Kconfig
index 1c02d23..9746fee 100644
--- a/drivers/gpu/drm/radeon/Kconfig
+++ b/drivers/gpu/drm/radeon/Kconfig
@@ -1,6 +1,7 @@
 config DRM_RADEON_KMS
bool Enable modesetting on radeon by default - NEW DRIVER
depends on DRM_RADEON
+   select BACKLIGHT_CLASS_DEVICE
help
  Choose this option if you want kernel modesetting enabled by default.
 
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c 
b/drivers/gpu/drm/radeon/radeon_connectors.c
index fe6c747..d20bc76 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -40,6 +40,10 @@ radeon_atombios_connected_scratch_regs(struct drm_connector 
*connector,
   struct drm_encoder *encoder,
   bool connected);
 
+extern void
+radeon_legacy_backlight_init(struct radeon_encoder *radeon_encoder,
+struct drm_connector *drm_connector);
+
 void radeon_connector_hotplug(struct drm_connector *connector)
 {
struct drm_device *dev = connector-dev;
@@ -1462,6 +1466,17 @@ radeon_add_legacy_connector(struct drm_device *dev,
connector-polled = DRM_CONNECTOR_POLL_HPD;
connector-display_info.subpixel_order = subpixel_order;
drm_sysfs_connector_add(connector);
+   if (connector_type == DRM_MODE_CONNECTOR_LVDS) {
+   struct drm_encoder *drm_encoder;
+
+   list_for_each_entry(drm_encoder, 
dev-mode_config.encoder_list, head) {
+   struct radeon_encoder *radeon_encoder;
+
+   radeon_encoder = to_radeon_encoder(drm_encoder);
+   if (radeon_encoder-encoder_id == 
ENCODER_OBJECT_ID_INTERNAL_LVDS)
+   radeon_legacy_backlight_init(radeon_encoder, 
connector);
+   }
+   }
return;
 
 failed:
diff --git a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c 
b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
index 0b83970..bdca317 100644
--- a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
+++ b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
@@ -28,6 +28,10 @@
 #include radeon_drm.h
 #include radeon.h
 #include atom.h
+#include linux/backlight.h
+#ifdef CONFIG_PMAC_BACKLIGHT
+#include asm/backlight.h
+#endif
 
 static void radeon_legacy_encoder_disable(struct drm_encoder *encoder)
 {
@@ -39,7 +43,7 @@ static void radeon_legacy_encoder_disable(struct drm_encoder 
*encoder)
radeon_encoder-active_device = 0;
 }
 
-static void radeon_legacy_lvds_dpms(struct drm_encoder *encoder, int mode)
+static void radeon_legacy_lvds_update(struct drm_encoder *encoder, int mode)
 {
struct drm_device *dev = encoder-dev;
struct radeon_device *rdev = dev-dev_private;
@@ -47,15 +51,23 @@ static void radeon_legacy_lvds_dpms(struct drm_encoder 
*encoder, int mode)
uint32_t lvds_gen_cntl, lvds_pll_cntl, pixclks_cntl, disp_pwr_man;
int panel_pwr_delay = 2000;
bool is_mac = false;
+   uint8_t backlight_level;
DRM_DEBUG_KMS(\n);
 
+   lvds_gen_cntl = RREG32(RADEON_LVDS_GEN_CNTL);
+   backlight_level = (lvds_gen_cntl  RADEON_LVDS_BL_MOD_LEVEL_SHIFT)  
0xff;
+
if (radeon_encoder-enc_priv) {
if (rdev-is_atom_bios) {
struct radeon_encoder_atom_dig *lvds = 
radeon_encoder-enc_priv;
panel_pwr_delay = lvds-panel_pwr_delay;
+   if (lvds-bl_dev)
+   backlight_level = lvds-backlight_level;
} else {
struct radeon_encoder_lvds *lvds = 
radeon_encoder-enc_priv;
panel_pwr_delay = lvds-panel_pwr_delay;
+   if (lvds-bl_dev)
+   backlight_level = lvds-backlight_level;
}
}
 
@@ -82,11 +94,13 @@ static void radeon_legacy_lvds_dpms(struct drm_encoder 
*encoder, int mode)
lvds_pll_cntl = ~RADEON_LVDS_PLL_RESET;
WREG32(RADEON_LVDS_PLL_CNTL, lvds_pll_cntl);
 
-   lvds_gen_cntl = RREG32(RADEON_LVDS_GEN_CNTL);
-   lvds_gen_cntl |= (RADEON_LVDS_ON | RADEON_LVDS_EN |

Re: [Nouveau] [PATCH 1/5] Backlight: Add backlight type

2010-11-22 Thread Richard Purdie 
On Fri, 2010-11-19 at 20:25 +, Matthew Garrett wrote:
 On Fri, Nov 19, 2010 at 12:05:23PM -0800, Andrew Morton wrote:
  On Fri, 19 Nov 2010 10:53:52 -0500
  Matthew Garrett m...@redhat.com wrote:
  
   There may be multiple ways of controlling the backlight on a given 
   machine.
   Allow drivers to expose the type of interface they are providing, making
   it possible for userspace to make appropriate policy decisions.
   
   ...
  
60 files changed, 102 insertions(+), 0 deletions(-)
  
  This patch has a pretty short half-life.
 
 Well, ideally it would have landed in the backlight tree when I sent it 
 months ago. Then we'd have the opportunity to ensure that everything was 
 fixed up before it went in in the merge window.
 
   @@ -62,6 +68,8 @@ struct backlight_properties {
 /* FB Blanking active? (values as for power) */
 /* Due to be removed, please use (state  BL_CORE_FBBLANK) */
 int fb_blank;
   + /* Backlight type */
   + enum backlight_type type;
 /* Flags used to signal drivers of state changes */
 /* Upper 4 bits are reserved for driver internal use */
 unsigned int state;
  
  And if/when the half-life expires, we'll have drivers in-tree which
  forget to set backlight_properties.type.  I haven't checked, but if
  we're lucky they will default to 0.
 
 Depends entirely on whether they kzalloc the structure or not before
 calling backlight_device_register(). 
 
  What will be the runtime effects upon such unconverted drivers? 
  Ideally we'd like them to continue to work OK, and to emit a runtime
  warning.  In which case you'll need BACKLIGHT_RAW=1 so the unconverted
  driver can be detected, warned about and fixed up by the core code.
 
 The worst case I can think of is that we walk off the array - I guess 
 there's an argument for sanity checking that in backlight_show_type().

I think adding a BACKLIGHT_TYPEUNKNOWN as the first item in the enum,
sanity checking the array bounds and printing a warning if type is not
one of the defined values would be good.

I also want to make sure you think this patch is going to scale with
multiple GPU output machines? Thats the main reason I've held off any
patch like this as it doesn't help solve that problem as far as I can
tell. Yes, we have the device parent information and I see later in the
patch series you ensure the backlight is registered against the
connector which is good. If you have an ACPI firmware control that you
say should always be preferred, how do we know which connector device
that corresponds to in the multiple output case? From that point of view
this model falls apart?

What I really want to avoid is a new interface which just papers over
cracks, only to have everything crumble anyway.

Cheers,

Richard



___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH 1/5] Backlight: Add backlight type

2010-11-22 Thread Richard Purdie 
On Mon, 2010-11-22 at 12:30 +, Matthew Garrett wrote:
 On Mon, Nov 22, 2010 at 10:17:00AM +, Richard Purdie wrote:
 
  I also want to make sure you think this patch is going to scale with
  multiple GPU output machines? Thats the main reason I've held off any
  patch like this as it doesn't help solve that problem as far as I can
  tell. Yes, we have the device parent information and I see later in the
  patch series you ensure the backlight is registered against the
  connector which is good. If you have an ACPI firmware control that you
  say should always be preferred, how do we know which connector device
  that corresponds to in the multiple output case? From that point of view
  this model falls apart?
 
 The ACPI device will point at the correct PCI device. Associating it 
 with the appropriate connector is theoretically possible in the case of 
 open drivers, but I hadn't seen it as a high priority since (in 
 practice) there's no situations where an ACPI interface will be able to 
 control more than one backlight.

Its the reverse situation I worry about. Are there situations where
there are multiple connectors on the PCI device and the ACPI interface
just controls one of them but controls for the other connectors may
exist?

I'm typing this with an external monitor plugged into my laptop with
i915 graphics...

Cheers,

Richard





___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau