ACPI: video: Don't call absent methods

PATCH In review Alexey Starikovskiy 4 participants 7 messages Sep 3, 2007 – Oct 2, 2007 ACPI

lore

Share & tools

lore

Alexey Starikovskiy

author

2007-09-03 12:30 UTC #0 lore raw

PATCH 1/2 ACPI: video: Don't call absent methods

Signed-off-by Ryan May <[email protected]>

Signed-off-by Alexey Starikovskiy <[email protected]>

---

 drivers/acpi/video.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

drivers/acpi/video.c +10 -8

index 3c9bb85..83aa41c 100644
@@ -409,14 +409,16 @@ acpi_video_device_lcd_query_levels(struct acpi_video_device *device,
 static int
 acpi_video_device_lcd_set_level(struct acpi_video_device *device, int level)
 {
-	int status;
+	int status = AE_OK;
 	union acpi_object arg0 = { ACPI_TYPE_INTEGER };
 	struct acpi_object_list args = { 1, &arg0 };
 
 
 	arg0.integer.value = level;
-	status = acpi_evaluate_object(device->dev->handle, "_BCM", &args, NULL);
-
+	if (device->cap._BCM)
+		status = acpi_evaluate_object(device->dev->handle, "_BCM",
+					      &args, NULL);
+	device->brightness->curr = level;
 	printk(KERN_DEBUG "set_level status: %x\n", status);
 	return status;
 }

@@ -425,11 +427,11 @@ static int
 acpi_video_device_lcd_get_level_current(struct acpi_video_device *device,
 					unsigned long *level)
 {
-	int status;
-
-	status = acpi_evaluate_integer(device->dev->handle, "_BQC", NULL, level);
-
-	return status;
+	if (device->cap._BQC)
+		return acpi_evaluate_integer(device->dev->handle, "_BQC", NULL,
+					     level);
+	*level = device->brightness->curr;
+	return AE_OK;
 }
 
 static int

Alexey Starikovskiy

author

2007-09-03 12:30 UTC #1 lore raw

PATCH 2/2 ACPI: VIDEO: Adjust current level to closest available one.

Signed-off-by Alexey Starikovskiy <[email protected]>

---

 drivers/acpi/video.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

drivers/acpi/video.c +12 -1

index 83aa41c..8a60a3d 100644
@@ -1636,9 +1636,20 @@ static int
 acpi_video_get_next_level(struct acpi_video_device *device,
 			  u32 level_current, u32 event)
 {
-	int min, max, min_above, max_below, i, l;
+	int min, max, min_above, max_below, i, l, delta = 255;
 	max = max_below = 0;
 	min = min_above = 255;
+	/* Find closest level to level_current */
+	for (i = 0; i < device->brightness->count; i++) {
+		l = device->brightness->levels[i];
+		if (abs(l - level_current) < abs(delta)) {
+			delta = l - level_current;
+			if (!delta)
+				break;
+		}
+	}
+	/* Ajust level_current to closest available level */
+	level_current += delta;
 	for (i = 0; i < device->brightness->count; i++) {
 		l = device->brightness->levels[i];
 		if (l < min)

Len Brown

reviewer

2007-09-03 14:57 UTC #2 lore raw

In reply to Alexey Starikovskiy

Is it a good idea to return AE_OK when no operation was actually performed?

-Len

Quoted 54 lines · Alexey Starikovskiy

Signed-off-by Ryan May <[email protected]>

Signed-off-by Alexey Starikovskiy <[email protected]>

---

 drivers/acpi/video.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

drivers/acpi/video.c +10 -9

index 3c9bb85..83aa41c 100644
@@ -409,14 +409,16 @@ acpi_video_device_lcd_query_levels(struct acpi_video_device *device,
 static int
 acpi_video_device_lcd_set_level(struct acpi_video_device *device, int level)
 {
-	int status;
+	int status = AE_OK;
 	union acpi_object arg0 = { ACPI_TYPE_INTEGER };
 	struct acpi_object_list args = { 1, &arg0 };
 
 
 	arg0.integer.value = level;
-	status = acpi_evaluate_object(device->dev->handle, "_BCM", &args, NULL);
-
+	if (device->cap._BCM)
+		status = acpi_evaluate_object(device->dev->handle, "_BCM",
+					      &args, NULL);
+	device->brightness->curr = level;
 	printk(KERN_DEBUG "set_level status: %x\n", status);
 	return status;
 }

@@ -425,11 +427,11 @@ static int
 acpi_video_device_lcd_get_level_current(struct acpi_video_device *device,
 					unsigned long *level)
 {
-	int status;
-
-	status = acpi_evaluate_integer(device->dev->handle, "_BQC", NULL, level);
-
-	return status;
+	if (device->cap._BQC)
+		return acpi_evaluate_integer(device->dev->handle, "_BQC", NULL,
+					     level);
+	*level = device->brightness->curr;
+	return AE_OK;
 }
 
 static int
 
-

To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Alexey Starikovskiy

2007-09-03 15:15 UTC #3 lore raw

Quoted 1 line · Len Brown

Is it a good idea to return AE_OK when no operation was actually performed?

There is a write into device->brightness->curr, so _some_ operation was performed. Do you suggest any other error code?

Alex

Quoted 63 lines

-Len

Quoted 50 lines · Alexey Starikovskiy

Signed-off-by Ryan May <[email protected]>

Signed-off-by Alexey Starikovskiy <[email protected]>

---

 drivers/acpi/video.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

drivers/acpi/video.c +10 -9

index 3c9bb85..83aa41c 100644
@@ -409,14 +409,16 @@ acpi_video_device_lcd_query_levels(struct acpi_video_device *device,
 static int
 acpi_video_device_lcd_set_level(struct acpi_video_device *device, int level)
 {
-	int status;
+	int status = AE_OK;
 	union acpi_object arg0 = { ACPI_TYPE_INTEGER };
 	struct acpi_object_list args = { 1, &arg0 };
 
 
 	arg0.integer.value = level;
-	status = acpi_evaluate_object(device->dev->handle, "_BCM", &args, NULL);
-
+	if (device->cap._BCM)
+		status = acpi_evaluate_object(device->dev->handle, "_BCM",
+					      &args, NULL);
+	device->brightness->curr = level;
 	printk(KERN_DEBUG "set_level status: %x\n", status);
 	return status;
 }

@@ -425,11 +427,11 @@ static int
 acpi_video_device_lcd_get_level_current(struct acpi_video_device *device,
 					unsigned long *level)
 {
-	int status;
-
-	status = acpi_evaluate_integer(device->dev->handle, "_BQC", NULL, level);
-
-	return status;
+	if (device->cap._BQC)
+		return acpi_evaluate_integer(device->dev->handle, "_BQC", NULL,
+					     level);
+	*level = device->brightness->curr;
+	return AE_OK;
 }
 
 static int
 
-

Quoted 4 lines

To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Ryan May

2007-09-14 14:01 UTC #4 lore raw

In reply to Alexey Starikovskiy

What's the status of this particular patch?

Ryan

Quoted 55 lines · Alexey Starikovskiy

Signed-off-by Ryan May <[email protected]>

Signed-off-by Alexey Starikovskiy <[email protected]>

---

 drivers/acpi/video.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

drivers/acpi/video.c +10 -9

index 3c9bb85..83aa41c 100644
@@ -409,14 +409,16 @@ acpi_video_device_lcd_query_levels(struct acpi_video_device *device,
 static int
 acpi_video_device_lcd_set_level(struct acpi_video_device *device, int level)
 {
-	int status;
+	int status = AE_OK;
 	union acpi_object arg0 = { ACPI_TYPE_INTEGER };
 	struct acpi_object_list args = { 1, &arg0 };
 
 
 	arg0.integer.value = level;
-	status = acpi_evaluate_object(device->dev->handle, "_BCM", &args, NULL);
-
+	if (device->cap._BCM)
+		status = acpi_evaluate_object(device->dev->handle, "_BCM",
+					      &args, NULL);
+	device->brightness->curr = level;
 	printk(KERN_DEBUG "set_level status: %x\n", status);
 	return status;
 }

@@ -425,11 +427,11 @@ static int
 acpi_video_device_lcd_get_level_current(struct acpi_video_device *device,
 					unsigned long *level)
 {
-	int status;
-
-	status = acpi_evaluate_integer(device->dev->handle, "_BQC", NULL, level);
-
-	return status;
+	if (device->cap._BQC)
+		return acpi_evaluate_integer(device->dev->handle, "_BQC", NULL,
+					     level);
+	*level = device->brightness->curr;
+	return AE_OK;
 }
 
 static int
 
-

To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 4 lines

Ryan May
Graduate Research Assistant
School of Meteorology
University of Oklahoma

Ryan May

2007-10-02 13:23 UTC #5 lore raw

In reply to Alexey Starikovskiy

Hi,

What's the status of this patch? AFAICT, it hasn't been applied upstream anywhere. This fixes the problems on my HP dv2125nr with the new BIOS, so I'd really like to see it merged.

Thanks,

Ryan

Quoted 55 lines · Alexey Starikovskiy

Signed-off-by Ryan May <[email protected]>

Signed-off-by Alexey Starikovskiy <[email protected]>

---

 drivers/acpi/video.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

drivers/acpi/video.c +10 -9

index 3c9bb85..83aa41c 100644
@@ -409,14 +409,16 @@ acpi_video_device_lcd_query_levels(struct acpi_video_device *device,
 static int
 acpi_video_device_lcd_set_level(struct acpi_video_device *device, int level)
 {
-	int status;
+	int status = AE_OK;
 	union acpi_object arg0 = { ACPI_TYPE_INTEGER };
 	struct acpi_object_list args = { 1, &arg0 };
 
 
 	arg0.integer.value = level;
-	status = acpi_evaluate_object(device->dev->handle, "_BCM", &args, NULL);
-
+	if (device->cap._BCM)
+		status = acpi_evaluate_object(device->dev->handle, "_BCM",
+					      &args, NULL);
+	device->brightness->curr = level;
 	printk(KERN_DEBUG "set_level status: %x\n", status);
 	return status;
 }

@@ -425,11 +427,11 @@ static int
 acpi_video_device_lcd_get_level_current(struct acpi_video_device *device,
 					unsigned long *level)
 {
-	int status;
-
-	status = acpi_evaluate_integer(device->dev->handle, "_BQC", NULL, level);
-
-	return status;
+	if (device->cap._BQC)
+		return acpi_evaluate_integer(device->dev->handle, "_BQC", NULL,
+					     level);
+	*level = device->brightness->curr;
+	return AE_OK;
 }
 
 static int
 
-

To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 4 lines

Ryan May
Graduate Research Assistant
School of Meteorology
University of Oklahoma

Alexey Starikovskiy

2007-10-02 15:53 UTC #6 lore raw

Ryan,
It is in queue for 2.6.24... Too late for .23

Regards,
Alex.

Quoted 68 lines · Ryan May

Hi,

What's the status of this patch? AFAICT, it hasn't been applied upstream anywhere. This fixes the problems on my HP dv2125nr with the new BIOS, so I'd really like to see it merged.

Thanks,

Ryan

Quoted 55 lines · Alexey Starikovskiy

Signed-off-by Ryan May <[email protected]>

Signed-off-by Alexey Starikovskiy <[email protected]>

---

 drivers/acpi/video.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

drivers/acpi/video.c +10 -9

index 3c9bb85..83aa41c 100644
@@ -409,14 +409,16 @@ acpi_video_device_lcd_query_levels(struct acpi_video_device *device,
 static int
 acpi_video_device_lcd_set_level(struct acpi_video_device *device, int level)
 {
-	int status;
+	int status = AE_OK;
 	union acpi_object arg0 = { ACPI_TYPE_INTEGER };
 	struct acpi_object_list args = { 1, &arg0 };
 
 
 	arg0.integer.value = level;
-	status = acpi_evaluate_object(device->dev->handle, "_BCM", &args, NULL);
-
+	if (device->cap._BCM)
+		status = acpi_evaluate_object(device->dev->handle, "_BCM",
+					      &args, NULL);
+	device->brightness->curr = level;
 	printk(KERN_DEBUG "set_level status: %x\n", status);
 	return status;
 }

@@ -425,11 +427,11 @@ static int
 acpi_video_device_lcd_get_level_current(struct acpi_video_device *device,
 					unsigned long *level)
 {
-	int status;
-
-	status = acpi_evaluate_integer(device->dev->handle, "_BQC", NULL, level);
-
-	return status;
+	if (device->cap._BQC)
+		return acpi_evaluate_integer(device->dev->handle, "_BQC", NULL,
+					     level);
+	*level = device->brightness->curr;
+	return AE_OK;
 }
 
 static int
 
-

To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html