diff mbox series

[v2,1/2] drm/nouveau/disp: add backlight for ada lovelace

Message ID 20240412194901.1596-1-angelo@kernel-space.org (mailing list archive)
State New, archived
Headers show
Series [v2,1/2] drm/nouveau/disp: add backlight for ada lovelace | expand

Commit Message

Angelo Dureghello April 12, 2024, 7:49 p.m. UTC 
Add working backlight for "ada lovelace" missing case.

The nvif method is actually not working for this chipset so
used the drm apis. Also, by dpcd, drm layer is calculating a
max brightness of 255, but to get a real correct max brightnes
the maximum must be multiplied by a factor of 16.

Tested to work properly in Legion Lenovo Pro 5

Lenovo Legion 5 Pro 16ARX8
Bios ver LPCN49WW
	 LPEC49WW
SN PF4T63AZ
Nvidia RTX4060 MaxQ/Mobile rev a1 (ADA LOVELACE AD107M)
AMD Ryzen 9 7945HX + Radeon

and wayland.

---
Changes for v2:
- add some comments
- remove x16 multiplication (hack)
- remove forgot debug printk

Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
---
 drivers/gpu/drm/nouveau/nouveau_backlight.c | 54 +++++++++++++++++++++
 1 file changed, 54 insertions(+)

Comments

Lyude Paul April 15, 2024, 4:54 p.m. UTC | #1 
Hm. Could you share some logs with drm.debug=0x116? I'm a bit confused
because I would have thought that we were already probing for eDP
backlights seeing as I added support for them at one point?

(I hope this isn't the point I learn I actually forgot to add support
for them :P)


On Fri, 2024-04-12 at 21:49 +0200, Angelo Dureghello wrote:
> Add working backlight for "ada lovelace" missing case.
> 
> The nvif method is actually not working for this chipset so
> used the drm apis. Also, by dpcd, drm layer is calculating a
> max brightness of 255, but to get a real correct max brightnes
> the maximum must be multiplied by a factor of 16.
> 
> Tested to work properly in Legion Lenovo Pro 5
> 
> Lenovo Legion 5 Pro 16ARX8
> Bios ver LPCN49WW
> 	 LPEC49WW
> SN PF4T63AZ
> Nvidia RTX4060 MaxQ/Mobile rev a1 (ADA LOVELACE AD107M)
> AMD Ryzen 9 7945HX + Radeon
> 
> and wayland.
> 
> ---
> Changes for v2:
> - add some comments
> - remove x16 multiplication (hack)
> - remove forgot debug printk
> 
> Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
> ---
>  drivers/gpu/drm/nouveau/nouveau_backlight.c | 54
> +++++++++++++++++++++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> index d47442125fa1..7b7306d18ad7 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> @@ -286,6 +286,56 @@ nv50_backlight_init(struct nouveau_backlight
> *bl,
>  	return 0;
>  }
>  
> +static int
> +nv19x_backlight_init(struct nouveau_backlight *bl,
> +		     struct nouveau_connector *nv_conn,
> +		     struct nouveau_encoder *nv_encoder,
> +		     struct backlight_properties *props,
> +		     const struct backlight_ops **ops)
> +{
> +	int ret;
> +	u16 current_level;
> +	u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
> +	u8 current_mode;
> +	struct nouveau_drm *drm = nouveau_drm(nv_encoder-
> >base.base.dev);
> +
> +	/*
> +	 * nvif functions, so also nvif_outp_bl_get, are not working
> with this
> +	 * connector (return -22), using only drm layer.
> +	 */
> +	if (nv_conn->type == DCB_CONNECTOR_eDP) {
> +
> +		ret = drm_dp_dpcd_read(&nv_conn->aux,
> DP_EDP_DPCD_REV, edp_dpcd,
> +				       EDP_DISPLAY_CTL_CAP_SIZE);
> +		if (ret < 0)
> +			return ret;
> +		if (!drm_edp_backlight_supported(edp_dpcd))
> +			return -ENODEV;
> +
> +		ret = drm_edp_backlight_init(&nv_conn->aux, &bl-
> >edp_info, 0, edp_dpcd,
> +					 &current_level,
> &current_mode);
> +		if (ret < 0)
> +			return ret;
> +
> +		ret = drm_edp_backlight_enable(&nv_conn->aux, &bl-
> >edp_info, current_level);
> +		if (ret < 0) {
> +			NV_ERROR(drm, "Failed to enable backlight on
> %s: %d\n",
> +				 nv_conn->base.name, ret);
> +			return ret;
> +		}
> +
> +		*ops = &nv50_edp_bl_ops;
> +
> +		props->max_brightness = bl->edp_info.max;
> +		props->brightness = current_level;
> +		bl->uses_dpcd = true;
> +
> +		return 0;
> +	}
> +
> +	return -ENODEV;
> +}
> +
>  int
>  nouveau_backlight_init(struct drm_connector *connector)
>  {
> @@ -332,6 +382,10 @@ nouveau_backlight_init(struct drm_connector
> *connector)
>  		ret = nv50_backlight_init(bl,
> nouveau_connector(connector),
>  					  nv_encoder, &props, &ops);
>  		break;
> +	case NV_DEVICE_INFO_V0_ADA:
> +		ret = nv19x_backlight_init(bl,
> nouveau_connector(connector),
> +					   nv_encoder, &props,
> &ops);
> +		break;
>  	default:
>  		ret = 0;
>  		goto fail_alloc;
Angelo Dureghello April 16, 2024, 7:11 a.m. UTC | #2 
Hi Lyude,

thanks a lot for your reply,

On 15/04/24 6:54 PM, Lyude Paul wrote:
> Hm. Could you share some logs with drm.debug=0x116? I'm a bit confused
> because I would have thought that we were already probing for eDP
> backlights seeing as I added support for them at one point?
>
> (I hope this isn't the point I learn I actually forgot to add support
> for them :P)
>
i am attaching 2 dmesg logs with drm.debug=0x116, with these 2 patches
applied to mainstream, and without them.

https://pastecode.io/s/fyjkky6y
https://pastecode.io/s/gdf8v3e3


Note, this laptop (Lenovo Legion 5 Pro 16ARX8) has 2 video cards,
amd (cpu built-in), and additional nvidia, it can work in "switchable"
mode, where amndgpu backlight works, and in discrete mode (as now),
using only nvidia gpu.

Main issue is that in nouveau_backlight.c
nouveau_backlight_init() is actually not considering ADA LOVELACE in
the main switch/case. So execution falls back to acpi backlight,
that for this laptop does not work at all (tried vendor, native,
video). Tested also nvidia-wmi-ec-backlight, the module does not
even get loaded. Only solution i found is this patchset.


> On Fri, 2024-04-12 at 21:49 +0200, Angelo Dureghello wrote:
>> Add working backlight for "ada lovelace" missing case.
>>
>> The nvif method is actually not working for this chipset so
>> used the drm apis. Also, by dpcd, drm layer is calculating a
>> max brightness of 255, but to get a real correct max brightnes
>> the maximum must be multiplied by a factor of 16.
>>
>> Tested to work properly in Legion Lenovo Pro 5
>>
>> Lenovo Legion 5 Pro 16ARX8
>> Bios ver LPCN49WW
>> 	 LPEC49WW
>> SN PF4T63AZ
>> Nvidia RTX4060 MaxQ/Mobile rev a1 (ADA LOVELACE AD107M)
>> AMD Ryzen 9 7945HX + Radeon
>>
>> and wayland.
>>
>> ---
>> Changes for v2:
>> - add some comments
>> - remove x16 multiplication (hack)
>> - remove forgot debug printk
>>
>> Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
>> ---
>>   drivers/gpu/drm/nouveau/nouveau_backlight.c | 54
>> +++++++++++++++++++++
>>   1 file changed, 54 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c
>> b/drivers/gpu/drm/nouveau/nouveau_backlight.c
>> index d47442125fa1..7b7306d18ad7 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
>> @@ -286,6 +286,56 @@ nv50_backlight_init(struct nouveau_backlight
>> *bl,
>>   	return 0;
>>   }
>>   
>> +static int
>> +nv19x_backlight_init(struct nouveau_backlight *bl,
>> +		     struct nouveau_connector *nv_conn,
>> +		     struct nouveau_encoder *nv_encoder,
>> +		     struct backlight_properties *props,
>> +		     const struct backlight_ops **ops)
>> +{
>> +	int ret;
>> +	u16 current_level;
>> +	u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
>> +	u8 current_mode;
>> +	struct nouveau_drm *drm = nouveau_drm(nv_encoder-
>>> base.base.dev);
>> +
>> +	/*
>> +	 * nvif functions, so also nvif_outp_bl_get, are not working
>> with this
>> +	 * connector (return -22), using only drm layer.
>> +	 */
>> +	if (nv_conn->type == DCB_CONNECTOR_eDP) {
>> +
>> +		ret = drm_dp_dpcd_read(&nv_conn->aux,
>> DP_EDP_DPCD_REV, edp_dpcd,
>> +				       EDP_DISPLAY_CTL_CAP_SIZE);
>> +		if (ret < 0)
>> +			return ret;
>> +		if (!drm_edp_backlight_supported(edp_dpcd))
>> +			return -ENODEV;
>> +
>> +		ret = drm_edp_backlight_init(&nv_conn->aux, &bl-
>>> edp_info, 0, edp_dpcd,
>> +					 &current_level,
>> &current_mode);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		ret = drm_edp_backlight_enable(&nv_conn->aux, &bl-
>>> edp_info, current_level);
>> +		if (ret < 0) {
>> +			NV_ERROR(drm, "Failed to enable backlight on
>> %s: %d\n",
>> +				 nv_conn->base.name, ret);
>> +			return ret;
>> +		}
>> +
>> +		*ops = &nv50_edp_bl_ops;
>> +
>> +		props->max_brightness = bl->edp_info.max;
>> +		props->brightness = current_level;
>> +		bl->uses_dpcd = true;
>> +
>> +		return 0;
>> +	}
>> +
>> +	return -ENODEV;
>> +}
>> +
>>   int
>>   nouveau_backlight_init(struct drm_connector *connector)
>>   {
>> @@ -332,6 +382,10 @@ nouveau_backlight_init(struct drm_connector
>> *connector)
>>   		ret = nv50_backlight_init(bl,
>> nouveau_connector(connector),
>>   					  nv_encoder, &props, &ops);
>>   		break;
>> +	case NV_DEVICE_INFO_V0_ADA:
>> +		ret = nv19x_backlight_init(bl,
>> nouveau_connector(connector),
>> +					   nv_encoder, &props,
>> &ops);
>> +		break;
>>   	default:
>>   		ret = 0;
>>   		goto fail_alloc;
> Regards,
> angelo
Moody Liu April 23, 2024, 6 p.m. UTC | #3 
Hello,

I would confirm that this patch works for my machine:

  01:00.0 VGA compatible controller: NVIDIA Corporation AD107M [GeForce RTX 4050 Max-Q / Mobile] (rev a1)
  01:00.1 Audio device: NVIDIA Corporation Device 22be (rev a1)

  System Information
    Manufacturer: LENOVO
    Product Name: 82WK
    Version: Legion Y9000P IRX8
    SKU Number: LENOVO_MT_82WK_BU_idea_FM_Legion Y9000P IRX8
    Family: Legion Y9000P IRX8

  Graphics Platform: Wayland
  Processors: 32 × 13th Gen Intel® Core™ i9-13900HX
  Memory: 62.6 GiB of RAM
  Graphics Processor: NV197
  Manufacturer: LENOVO
  Product Name: 82WK
  System Version: Legion Y9000P IRX8


Moody Liu
diff mbox series

Patch

diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index d47442125fa1..7b7306d18ad7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -286,6 +286,56 @@  nv50_backlight_init(struct nouveau_backlight *bl,
 	return 0;
 }
 
+static int
+nv19x_backlight_init(struct nouveau_backlight *bl,
+		     struct nouveau_connector *nv_conn,
+		     struct nouveau_encoder *nv_encoder,
+		     struct backlight_properties *props,
+		     const struct backlight_ops **ops)
+{
+	int ret;
+	u16 current_level;
+	u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
+	u8 current_mode;
+	struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
+
+	/*
+	 * nvif functions, so also nvif_outp_bl_get, are not working with this
+	 * connector (return -22), using only drm layer.
+	 */
+	if (nv_conn->type == DCB_CONNECTOR_eDP) {
+
+		ret = drm_dp_dpcd_read(&nv_conn->aux, DP_EDP_DPCD_REV, edp_dpcd,
+				       EDP_DISPLAY_CTL_CAP_SIZE);
+		if (ret < 0)
+			return ret;
+		if (!drm_edp_backlight_supported(edp_dpcd))
+			return -ENODEV;
+
+		ret = drm_edp_backlight_init(&nv_conn->aux, &bl->edp_info, 0, edp_dpcd,
+					 &current_level, &current_mode);
+		if (ret < 0)
+			return ret;
+
+		ret = drm_edp_backlight_enable(&nv_conn->aux, &bl->edp_info, current_level);
+		if (ret < 0) {
+			NV_ERROR(drm, "Failed to enable backlight on %s: %d\n",
+				 nv_conn->base.name, ret);
+			return ret;
+		}
+
+		*ops = &nv50_edp_bl_ops;
+
+		props->max_brightness = bl->edp_info.max;
+		props->brightness = current_level;
+		bl->uses_dpcd = true;
+
+		return 0;
+	}
+
+	return -ENODEV;
+}
+
 int
 nouveau_backlight_init(struct drm_connector *connector)
 {
@@ -332,6 +382,10 @@  nouveau_backlight_init(struct drm_connector *connector)
 		ret = nv50_backlight_init(bl, nouveau_connector(connector),
 					  nv_encoder, &props, &ops);
 		break;
+	case NV_DEVICE_INFO_V0_ADA:
+		ret = nv19x_backlight_init(bl, nouveau_connector(connector),
+					   nv_encoder, &props, &ops);
+		break;
 	default:
 		ret = 0;
 		goto fail_alloc;