From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f51.google.com (mail-pj1-f51.google.com [209.85.216.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8579B364E85 for ; Wed, 20 May 2026 10:21:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.51 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779272482; cv=none; b=Eq5TJmB135ZQl9T5ekbYQLJnMJTLR1iRJ1tYFSytgWFgQ9HOnMsUGbda7x1cWFZJ0x7VnapbSsrCaAFpKsInCY2G4/PUJ1jjDq/7lHxw2uV1g1zeuOS04dAa5BCMs0lisLnNf6HYDfukDoUBYz1eJmN31FCUOHyqIuyPCt0797o= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779272482; c=relaxed/simple; bh=2FMOcR6JfTuw8Vq4YJAeYGTgqH7XoUOVE9FZUbr5Mas=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=AEP4X478O0H3XOBWzHYAhnWU9brXyp1OtAaa64LcIYqmzOEa/PhxoF/rtVkNisSD+kjb4xcgRBXbv3S2IKnTrmrxtGGAjmefdL/YHbjX/0agB5blyPp0PXq8PP2VlvGp+70vmwpYp6MCjZx9GWVfUHOVennCLI4yeJWXteqTjDc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=YJ/g5wEo; arc=none smtp.client-ip=209.85.216.51 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="YJ/g5wEo" Received: by mail-pj1-f51.google.com with SMTP id 98e67ed59e1d1-365cae89bf5so1900350a91.3 for ; Wed, 20 May 2026 03:21:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1779272479; x=1779877279; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=KPqLRcXHSDXYbacK9Af3pl+uowBsWOkNsTTCiwv4b3c=; b=YJ/g5wEo4zvJNVhpYSzw93+zlhSBta8kK1bbedu4X1OF+rE4TodYGo+l4PQU/iDrIM SPJZqOTn3PeOs0uCMMkp9etIOFx8bmlWzvS80ydoyt5fT93HSonibhF/MNyPdWnfw+cP VdlDp8gzBM4a7ZeKHg4/lZYOfzsIKVSb+tN+1m7PnM0ARxPOp8BkaWoA9slUew2bMrb7 OjQSgnoNuZQDSLaVelkMM1d9C+wY/tYg4i3s5EcPuXfCdEVBw8xyKrBodTBfK6CU18NG 9OygxgabEmr6GLFki78GXv7UZQ1eDXxtUvuNmIn/Dni0qi6njEIzEHN5VRmx5Ro44MyV bPqw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779272479; x=1779877279; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=KPqLRcXHSDXYbacK9Af3pl+uowBsWOkNsTTCiwv4b3c=; b=PXdtS8U/uQL6BtEH0kaL+sVPHwhVCcCRLEJ0fIz35unHv8CzeQtAuCa+RjQOIOcZbY FsYKIStO85SfqCZw1AXiW2wyKdD6LgEaqDNKgSV2qB86IKnWEIuoq6ReNL4nqaiuxsD5 TbnpAQ0N9p/QCMtWE5+jYhznGByKXjccCvG84CK0Afa1LjCG2yMoEm0EdMs5eyHiv6xH XFbRZS+XXKtdsqXQAypWWw3ZMLqDz+DajTZyUEtfTjGOtVr3YKsQbjA1nyM0htQ7VNwH w54zTV+uoA5uo/qAsUyTp1beNkbc3qdVDf9dvyNuu2iVs9cJlRXwdZr6YzsJzENtlGwX OA1A== X-Forwarded-Encrypted: i=1; AFNElJ874wyxhM5ZWhn2UUtseb6F/PCchfBXbE6gEwSau3psjx7/hOuKorfH3TfiyERFM0TEGF96yTFb3Yl+cEQ=@vger.kernel.org X-Gm-Message-State: AOJu0YwScITWxRGzFl6ATk4jfPLKD1MbXEqaydpQg/byH/lEFWyV2jVG 32b/fB9o1NS8j2rzw/Lc2gCZ8c4kTRkwv8CWcyC/dap0XzrO8qG5jApkI2+G3Q== X-Gm-Gg: Acq92OGoJGhr7Rls+zwHJUstjRMGhj0d1sbzUekGkUB9g65Wgn+TIXJWxqFYSFLmIaP vUx3EiaBxxOb44k0zJDb3Yddsuuw8ERAuGkTll22FjsUokjry2IW6jFZ4PH+UuRNCvtU23OQTFz ACX/kX5SXx1b8k6pj8u0VINoitQcG/bIk9ch808QNWOAEWmJauH8sLO6Ud+YNUIdur1mRLITKjb 1fFwKJ5Wxu7gkrHJ/53NNUMDSNwQQAcey9JMZcTRE3eVMS2ILpro+16PoPQit2LSltt+mTik/cE VvaJxDZmJ4FzUVe4FsrPC9QUaIXptLl8ufQUgdjZlRshJcBmoRkRtDyX+Gz8Rewxs12bBa9nNLI Hw6/ezoSeSgmyoXMb7FvV8S8JdxirMT4f/go4BK0i/PlVApz8lO54347phUVH00v7qJALgzOk3V aXquOF6lUV5LPCx9h9uaWUp8rMGdzbK5ugL5XVkXM97XiLB50EDbk+rr6NdHj6ppIaPp/OL4Q6Z 0zNxhMD+kUUDXPu8xca X-Received: by 2002:a17:90b:57ee:b0:368:147b:536a with SMTP id 98e67ed59e1d1-36951b97c8cmr24756505a91.14.1779272478654; Wed, 20 May 2026 03:21:18 -0700 (PDT) Received: from archlinux ([2405:201:1b:225f:36f2:f474:be1d:cad7]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-369512424b2sm16856582a91.3.2026.05.20.03.21.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 May 2026 03:21:18 -0700 (PDT) Date: Wed, 20 May 2026 15:51:14 +0530 From: Krishna Chomal To: foobisdweik Cc: Hans de Goede , Ilpo =?utf-8?B?SsOkcnZpbmVu?= , platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] platform/x86: hp-wmi: Expose independent CPU/GPU pwm channels Message-ID: References: <20260513193916.84673-1-dweikmferris@gmail.com> <20260513193916.84673-3-dweikmferris@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Disposition: inline In-Reply-To: <20260513193916.84673-3-dweikmferris@gmail.com> On Wed, May 13, 2026 at 12:39:16PM -0700, foobisdweik wrote: >The Victus-S WMI fan-speed-set query (HPWMI_VICTUS_S_FAN_SPEED_SET_QUERY, >GM2E in firmware) takes a two-byte buffer [cpu, gpu] and writes the >values to EC SRP1/SRP2 setpoints independently. The driver however drove >both fans from a single hwmon pwm1 attribute, with the GPU value >derived as cpu + priv->gpu_delta from the fan table. This makes >asymmetric fan curves (common ask: GPU fan louder than CPU under >heavy gaming load) impossible from userspace fan-control tools that >expect one pwm attribute per fan. > >Promote priv->pwm to a two-element array, add a second pwm hwmon >channel, and extend hp_wmi_hwmon_write() / hp_wmi_apply_fan_settings() >to drive the per-channel setpoints through GM2E. > >The U8_MAX sentinel in hp_wmi_fan_speed_set() selects the new >per-fan path; existing callers using HP_FAN_SPEED_AUTOMATIC or a >direct rpm value keep their behavior. The second pwm channel is only >visible on Victus-S-capable boards, since other HP laptops still use >the legacy single-fan WMI commands. > >Tested on OMEN by HP Laptop 16-b1xxx (8A13): writing pwm1=60, pwm2=200 >in manual mode drives CPU fan to ~1600 RPM and GPU fan to ~5700 RPM >simultaneously, verified via fan1_input/fan2_input tachs and direct >read of EC offsets 0xB0..0xB3. > >Signed-off-by: foobisdweik >--- > drivers/platform/x86/hp/hp-wmi.c | 45 +++++++++++++++++++++++--------- > 1 file changed, 32 insertions(+), 13 deletions(-) > >diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c >index 389506a6d2e3..40eb3715583a 100644 >--- a/drivers/platform/x86/hp/hp-wmi.c >+++ b/drivers/platform/x86/hp/hp-wmi.c >@@ -486,7 +486,7 @@ struct hp_wmi_hwmon_priv { > u8 max_rpm; > int gpu_delta; > u8 mode; >- u8 pwm; >+ u8 pwm[2]; > struct delayed_work keep_alive_dwork; > }; > >@@ -822,12 +822,18 @@ static int hp_wmi_fan_speed_set(struct hp_wmi_hwmon_priv *priv, u8 speed) > fan_speed[GPU_FAN] = speed; > > /* >- * GPU fan speed is always a little higher than CPU fan speed, we fetch >- * this delta value from the fan table during hwmon init. >- * Exception: Speed is set to HP_FAN_SPEED_AUTOMATIC, to revert to >- * automatic mode. >+ * Pass-through value U8_MAX: drive each fan from its own >+ * priv->pwm[] setpoint converted via pwm_to_rpm(). Used by the >+ * hwmon pwm1/pwm2 path that allows independent CPU/GPU fan control. >+ * >+ * Otherwise: GPU fan speed is always a little higher than CPU fan >+ * speed; we fetch this delta from the fan table during hwmon init. >+ * Exception: HP_FAN_SPEED_AUTOMATIC reverts to automatic mode. > */ >- if (speed != HP_FAN_SPEED_AUTOMATIC) { >+ if (speed == U8_MAX) { >+ fan_speed[CPU_FAN] = pwm_to_rpm(priv->pwm[CPU_FAN], priv); >+ fan_speed[GPU_FAN] = pwm_to_rpm(priv->pwm[GPU_FAN], priv); >+ } else if (speed != HP_FAN_SPEED_AUTOMATIC) { > gpu_speed = speed + priv->gpu_delta; > fan_speed[GPU_FAN] = clamp_val(gpu_speed, 0, U8_MAX); > } Currently, there are 2 callers for hp_wmi_fan_speed_set: 1. hp_wmi_fan_speed_reset() -> calls via speed = HP_FAN_SPEED_AUTOMATIC, and 2. hp_wmi_apply_fan_settings() -> calls via speed = U8_MAX (as updated by you) So this else-if block is effectively never executed, which in turn means that gpu_delta is also now redundant. >@@ -2398,7 +2404,7 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv) > case PWM_MODE_MANUAL: > if (!is_victus_s_thermal_profile()) > return -EOPNOTSUPP; >- ret = hp_wmi_fan_speed_set(priv, pwm_to_rpm(priv->pwm, priv)); >+ ret = hp_wmi_fan_speed_set(priv, U8_MAX); I think you could avoid passing the sentinel argument (speed) and directly update priv->pwm[] in the caller: hp_wmi_fan_speed_reset() > if (ret < 0) > return ret; > mod_delayed_work(system_wq, &priv->keep_alive_dwork, >@@ -2429,6 +2435,12 @@ static umode_t hp_wmi_hwmon_is_visible(const void *data, > { > switch (type) { > case hwmon_pwm: >+ /* >+ * Second pwm channel only exists on Victus-S-style boards >+ * which expose an independent GPU fan setpoint. >+ */ >+ if (channel == 1 && !is_victus_s_thermal_profile()) >+ return 0; Technically, the fan table has a dedicated field .num_fans (see struct victus_s_fan_table_header), so there is a possibility that some Victus-S-style device may not have second fan for pwm channel. But at least for the current entries, this is not the case, so I guess it would be safe to keep it this way. > if (attr == hwmon_pwm_input && !is_victus_s_thermal_profile()) > return 0; > return 0644; >@@ -2514,7 +2526,7 @@ static int hp_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type, > /* ensure PWM input is within valid fan speeds */ > rpm = pwm_to_rpm(val, priv); > rpm = clamp_val(rpm, priv->min_rpm, priv->max_rpm); >- priv->pwm = rpm_to_pwm(rpm, priv); >+ priv->pwm[channel] = rpm_to_pwm(rpm, priv); > return hp_wmi_apply_fan_settings(priv); > } > switch (val) { >@@ -2525,13 +2537,18 @@ static int hp_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type, > if (!is_victus_s_thermal_profile()) > return -EOPNOTSUPP; > /* >- * When switching to manual mode, set fan speed to >- * current RPM values to ensure a smooth transition. >+ * When switching to manual mode, seed each per-fan >+ * setpoint from its current measured RPM so the >+ * transition is smooth. > */ >- rpm = hp_wmi_get_fan_speed_victus_s(channel); >+ rpm = hp_wmi_get_fan_speed_victus_s(CPU_FAN); >+ if (rpm < 0) >+ return rpm; >+ priv->pwm[CPU_FAN] = rpm_to_pwm(rpm / 100, priv); >+ rpm = hp_wmi_get_fan_speed_victus_s(GPU_FAN); > if (rpm < 0) > return rpm; >- priv->pwm = rpm_to_pwm(rpm / 100, priv); >+ priv->pwm[GPU_FAN] = rpm_to_pwm(rpm / 100, priv); > priv->mode = PWM_MODE_MANUAL; > return hp_wmi_apply_fan_settings(priv); > case PWM_MODE_AUTO: >@@ -2547,7 +2564,9 @@ static int hp_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type, > > static const struct hwmon_channel_info * const info[] = { > HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT, HWMON_F_INPUT), >- HWMON_CHANNEL_INFO(pwm, HWMON_PWM_ENABLE | HWMON_PWM_INPUT), >+ HWMON_CHANNEL_INFO(pwm, >+ HWMON_PWM_ENABLE | HWMON_PWM_INPUT, >+ HWMON_PWM_ENABLE | HWMON_PWM_INPUT), I think you can take this opportunity to explore adding fanX_{min,max,input} to hwmon as well. > NULL > }; > >-- >2.54.0 >