All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Alexey Lukyanchuk <skif@skif-web.ru>, tvrtko.ursulin@linux.intel.com
Cc: Alexey Lukyanchuk <skif@skif-web.ru>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH RFC] fix dell wyse 3040 poweroff
Date: Mon, 12 Dec 2022 11:15:19 +0200	[thread overview]
Message-ID: <87y1rdx9so.fsf@intel.com> (raw)
In-Reply-To: <20221210180118.22087-1-skif@skif-web.ru>

On Sat, 10 Dec 2022, Alexey Lukyanchuk <skif@skif-web.ru> wrote:
> Dell wyse 3040 cat't poweroff aftet kernel 5.11.
> It happens  because i915_driver_shutdown function.
> Disabling of this function mitigate this problem.
>
> Fixes: 440b354f3 ("drivers/gpu/drm:power off troubles on dell wyse 3040") 

Fixes: is supposed to reference an existing commit.

> Signed-off-by: Alexey Lukyanchuk <skif@skif-web.ru> 
> ---
> There is trouble with i915_driver_shutdown function. After some diving I found that trouble looks like race condition in drm_atomic_get_connector_state function (drivers/gpu/drm/drm_atomic.c), maybe it linked to iterators. Now I fully exclude i915_driver_shutdown for wyse 3040 device.
>
> Can any one comment on this one please ? 

Bypassing the entire shutdown function is not an acceptable quirk.

Please file a bug over at fdo gitlab [1]. Add drm.debug=0xe module
parameter, and attach dmesg from boot to reproducing the problem. Add
log_buf_len=8M or similar as necessary to get the complete dmesg.

Have you tried the more recent kernels?

BR,
Jani.


[1] https://gitlab.freedesktop.org/drm/intel/wikis/How-to-file-i915-bugs


> ---
>  drivers/gpu/drm/i915/display/intel_quirks.c | 25 +++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_driver.c          |  3 +++
>  drivers/gpu/drm/i915/i915_drv.h             |  1 +
>  3 files changed, 29 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c b/drivers/gpu/drm/i915/display/intel_quirks.c
> index e415cd7c0..a6a549d48 100644
> --- a/drivers/gpu/drm/i915/display/intel_quirks.c
> +++ b/drivers/gpu/drm/i915/display/intel_quirks.c
> @@ -60,6 +60,12 @@ static void quirk_no_pps_backlight_power_hook(struct drm_i915_private *i915)
>  	drm_info(&i915->drm, "Applying no pps backlight power quirk\n");
>  }
>  
> +static void quirk_wyse_3040_shutdown_fix(struct drm_i915_private *i915)
> +{
> +	i915->quirks |= QUIRK_WYSE_3040_SHUTDOWN_FIX;
> +	drm_info(&i915->drm, "Applying wyse 3040 shutdown fix\n");
> +}
> +
>  struct intel_quirk {
>  	int device;
>  	int subsystem_vendor;
> @@ -85,6 +91,12 @@ static int intel_dmi_no_pps_backlight(const struct dmi_system_id *id)
>  	return 1;
>  }
>  
> +static int wyse_3040_shutdown_fix(const struct dmi_system_id *id)
> +{
> +	DRM_INFO("This device need help with poweroff %s\n", id->ident);
> +	return 1;
> +}
> +
>  static const struct intel_dmi_quirk intel_dmi_quirks[] = {
>  	{
>  		.dmi_id_list = &(const struct dmi_system_id[]) {
> @@ -131,6 +143,19 @@ static const struct intel_dmi_quirk intel_dmi_quirks[] = {
>  		},
>  		.hook = quirk_no_pps_backlight_power_hook,
>  	},
> +	{
> +		.dmi_id_list = &(const struct dmi_system_id[]) {
> +			{
> +				.callback = wyse_3040_shutdown_fix,
> +				.ident = "Dell Inc. 0G56C0",
> +				.matches = {DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Dell Inc."),
> +					    DMI_EXACT_MATCH(DMI_BOARD_NAME, "0G56C0"),
> +				},
> +			},
> +			{ }
> +		},
> +		.hook = quirk_wyse_3040_shutdown_fix,
> +	},
>  };
>  
>  static struct intel_quirk intel_quirks[] = {
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index deb8a8b76..af60fb79a 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1079,6 +1079,9 @@ static void intel_shutdown_encoders(struct drm_i915_private *dev_priv)
>  
>  void i915_driver_shutdown(struct drm_i915_private *i915)
>  {
> +	if (!(i915->quirks & QUIRK_WYSE_3040_SHUTDOWN_FIX))
> +		return;
> +
>  	disable_rpm_wakeref_asserts(&i915->runtime_pm);
>  	intel_runtime_pm_disable(&i915->runtime_pm);
>  	intel_power_domains_disable(i915);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 086bbe894..fdd6866e7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -200,6 +200,7 @@ struct drm_i915_display_funcs {
>  #define QUIRK_INCREASE_T12_DELAY (1<<6)
>  #define QUIRK_INCREASE_DDI_DISABLED_TIME (1<<7)
>  #define QUIRK_NO_PPS_BACKLIGHT_POWER_HOOK (1<<8)
> +#define QUIRK_WYSE_3040_SHUTDOWN_FIX (1<<9)
>  
>  struct i915_suspend_saved_registers {
>  	u32 saveDSPARB;

-- 
Jani Nikula, Intel Open Source Graphics Center

WARNING: multiple messages have this Message-ID (diff)
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Alexey Lukyanchuk <skif@skif-web.ru>, tvrtko.ursulin@linux.intel.com
Cc: Alexey Lukyanchuk <skif@skif-web.ru>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [PATCH RFC] fix dell wyse 3040 poweroff
Date: Mon, 12 Dec 2022 11:15:19 +0200	[thread overview]
Message-ID: <87y1rdx9so.fsf@intel.com> (raw)
In-Reply-To: <20221210180118.22087-1-skif@skif-web.ru>

On Sat, 10 Dec 2022, Alexey Lukyanchuk <skif@skif-web.ru> wrote:
> Dell wyse 3040 cat't poweroff aftet kernel 5.11.
> It happens  because i915_driver_shutdown function.
> Disabling of this function mitigate this problem.
>
> Fixes: 440b354f3 ("drivers/gpu/drm:power off troubles on dell wyse 3040") 

Fixes: is supposed to reference an existing commit.

> Signed-off-by: Alexey Lukyanchuk <skif@skif-web.ru> 
> ---
> There is trouble with i915_driver_shutdown function. After some diving I found that trouble looks like race condition in drm_atomic_get_connector_state function (drivers/gpu/drm/drm_atomic.c), maybe it linked to iterators. Now I fully exclude i915_driver_shutdown for wyse 3040 device.
>
> Can any one comment on this one please ? 

Bypassing the entire shutdown function is not an acceptable quirk.

Please file a bug over at fdo gitlab [1]. Add drm.debug=0xe module
parameter, and attach dmesg from boot to reproducing the problem. Add
log_buf_len=8M or similar as necessary to get the complete dmesg.

Have you tried the more recent kernels?

BR,
Jani.


[1] https://gitlab.freedesktop.org/drm/intel/wikis/How-to-file-i915-bugs


> ---
>  drivers/gpu/drm/i915/display/intel_quirks.c | 25 +++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_driver.c          |  3 +++
>  drivers/gpu/drm/i915/i915_drv.h             |  1 +
>  3 files changed, 29 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c b/drivers/gpu/drm/i915/display/intel_quirks.c
> index e415cd7c0..a6a549d48 100644
> --- a/drivers/gpu/drm/i915/display/intel_quirks.c
> +++ b/drivers/gpu/drm/i915/display/intel_quirks.c
> @@ -60,6 +60,12 @@ static void quirk_no_pps_backlight_power_hook(struct drm_i915_private *i915)
>  	drm_info(&i915->drm, "Applying no pps backlight power quirk\n");
>  }
>  
> +static void quirk_wyse_3040_shutdown_fix(struct drm_i915_private *i915)
> +{
> +	i915->quirks |= QUIRK_WYSE_3040_SHUTDOWN_FIX;
> +	drm_info(&i915->drm, "Applying wyse 3040 shutdown fix\n");
> +}
> +
>  struct intel_quirk {
>  	int device;
>  	int subsystem_vendor;
> @@ -85,6 +91,12 @@ static int intel_dmi_no_pps_backlight(const struct dmi_system_id *id)
>  	return 1;
>  }
>  
> +static int wyse_3040_shutdown_fix(const struct dmi_system_id *id)
> +{
> +	DRM_INFO("This device need help with poweroff %s\n", id->ident);
> +	return 1;
> +}
> +
>  static const struct intel_dmi_quirk intel_dmi_quirks[] = {
>  	{
>  		.dmi_id_list = &(const struct dmi_system_id[]) {
> @@ -131,6 +143,19 @@ static const struct intel_dmi_quirk intel_dmi_quirks[] = {
>  		},
>  		.hook = quirk_no_pps_backlight_power_hook,
>  	},
> +	{
> +		.dmi_id_list = &(const struct dmi_system_id[]) {
> +			{
> +				.callback = wyse_3040_shutdown_fix,
> +				.ident = "Dell Inc. 0G56C0",
> +				.matches = {DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Dell Inc."),
> +					    DMI_EXACT_MATCH(DMI_BOARD_NAME, "0G56C0"),
> +				},
> +			},
> +			{ }
> +		},
> +		.hook = quirk_wyse_3040_shutdown_fix,
> +	},
>  };
>  
>  static struct intel_quirk intel_quirks[] = {
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index deb8a8b76..af60fb79a 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1079,6 +1079,9 @@ static void intel_shutdown_encoders(struct drm_i915_private *dev_priv)
>  
>  void i915_driver_shutdown(struct drm_i915_private *i915)
>  {
> +	if (!(i915->quirks & QUIRK_WYSE_3040_SHUTDOWN_FIX))
> +		return;
> +
>  	disable_rpm_wakeref_asserts(&i915->runtime_pm);
>  	intel_runtime_pm_disable(&i915->runtime_pm);
>  	intel_power_domains_disable(i915);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 086bbe894..fdd6866e7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -200,6 +200,7 @@ struct drm_i915_display_funcs {
>  #define QUIRK_INCREASE_T12_DELAY (1<<6)
>  #define QUIRK_INCREASE_DDI_DISABLED_TIME (1<<7)
>  #define QUIRK_NO_PPS_BACKLIGHT_POWER_HOOK (1<<8)
> +#define QUIRK_WYSE_3040_SHUTDOWN_FIX (1<<9)
>  
>  struct i915_suspend_saved_registers {
>  	u32 saveDSPARB;

-- 
Jani Nikula, Intel Open Source Graphics Center

WARNING: multiple messages have this Message-ID (diff)
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Alexey Lukyanchuk <skif@skif-web.ru>, tvrtko.ursulin@linux.intel.com
Cc: Alexey Lukyanchuk <skif@skif-web.ru>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org, Daniel Vetter <daniel@ffwll.ch>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	David Airlie <airlied@gmail.com>
Subject: Re: [Intel-gfx] [PATCH RFC] fix dell wyse 3040 poweroff
Date: Mon, 12 Dec 2022 11:15:19 +0200	[thread overview]
Message-ID: <87y1rdx9so.fsf@intel.com> (raw)
In-Reply-To: <20221210180118.22087-1-skif@skif-web.ru>

On Sat, 10 Dec 2022, Alexey Lukyanchuk <skif@skif-web.ru> wrote:
> Dell wyse 3040 cat't poweroff aftet kernel 5.11.
> It happens  because i915_driver_shutdown function.
> Disabling of this function mitigate this problem.
>
> Fixes: 440b354f3 ("drivers/gpu/drm:power off troubles on dell wyse 3040") 

Fixes: is supposed to reference an existing commit.

> Signed-off-by: Alexey Lukyanchuk <skif@skif-web.ru> 
> ---
> There is trouble with i915_driver_shutdown function. After some diving I found that trouble looks like race condition in drm_atomic_get_connector_state function (drivers/gpu/drm/drm_atomic.c), maybe it linked to iterators. Now I fully exclude i915_driver_shutdown for wyse 3040 device.
>
> Can any one comment on this one please ? 

Bypassing the entire shutdown function is not an acceptable quirk.

Please file a bug over at fdo gitlab [1]. Add drm.debug=0xe module
parameter, and attach dmesg from boot to reproducing the problem. Add
log_buf_len=8M or similar as necessary to get the complete dmesg.

Have you tried the more recent kernels?

BR,
Jani.


[1] https://gitlab.freedesktop.org/drm/intel/wikis/How-to-file-i915-bugs


> ---
>  drivers/gpu/drm/i915/display/intel_quirks.c | 25 +++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_driver.c          |  3 +++
>  drivers/gpu/drm/i915/i915_drv.h             |  1 +
>  3 files changed, 29 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c b/drivers/gpu/drm/i915/display/intel_quirks.c
> index e415cd7c0..a6a549d48 100644
> --- a/drivers/gpu/drm/i915/display/intel_quirks.c
> +++ b/drivers/gpu/drm/i915/display/intel_quirks.c
> @@ -60,6 +60,12 @@ static void quirk_no_pps_backlight_power_hook(struct drm_i915_private *i915)
>  	drm_info(&i915->drm, "Applying no pps backlight power quirk\n");
>  }
>  
> +static void quirk_wyse_3040_shutdown_fix(struct drm_i915_private *i915)
> +{
> +	i915->quirks |= QUIRK_WYSE_3040_SHUTDOWN_FIX;
> +	drm_info(&i915->drm, "Applying wyse 3040 shutdown fix\n");
> +}
> +
>  struct intel_quirk {
>  	int device;
>  	int subsystem_vendor;
> @@ -85,6 +91,12 @@ static int intel_dmi_no_pps_backlight(const struct dmi_system_id *id)
>  	return 1;
>  }
>  
> +static int wyse_3040_shutdown_fix(const struct dmi_system_id *id)
> +{
> +	DRM_INFO("This device need help with poweroff %s\n", id->ident);
> +	return 1;
> +}
> +
>  static const struct intel_dmi_quirk intel_dmi_quirks[] = {
>  	{
>  		.dmi_id_list = &(const struct dmi_system_id[]) {
> @@ -131,6 +143,19 @@ static const struct intel_dmi_quirk intel_dmi_quirks[] = {
>  		},
>  		.hook = quirk_no_pps_backlight_power_hook,
>  	},
> +	{
> +		.dmi_id_list = &(const struct dmi_system_id[]) {
> +			{
> +				.callback = wyse_3040_shutdown_fix,
> +				.ident = "Dell Inc. 0G56C0",
> +				.matches = {DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Dell Inc."),
> +					    DMI_EXACT_MATCH(DMI_BOARD_NAME, "0G56C0"),
> +				},
> +			},
> +			{ }
> +		},
> +		.hook = quirk_wyse_3040_shutdown_fix,
> +	},
>  };
>  
>  static struct intel_quirk intel_quirks[] = {
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index deb8a8b76..af60fb79a 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1079,6 +1079,9 @@ static void intel_shutdown_encoders(struct drm_i915_private *dev_priv)
>  
>  void i915_driver_shutdown(struct drm_i915_private *i915)
>  {
> +	if (!(i915->quirks & QUIRK_WYSE_3040_SHUTDOWN_FIX))
> +		return;
> +
>  	disable_rpm_wakeref_asserts(&i915->runtime_pm);
>  	intel_runtime_pm_disable(&i915->runtime_pm);
>  	intel_power_domains_disable(i915);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 086bbe894..fdd6866e7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -200,6 +200,7 @@ struct drm_i915_display_funcs {
>  #define QUIRK_INCREASE_T12_DELAY (1<<6)
>  #define QUIRK_INCREASE_DDI_DISABLED_TIME (1<<7)
>  #define QUIRK_NO_PPS_BACKLIGHT_POWER_HOOK (1<<8)
> +#define QUIRK_WYSE_3040_SHUTDOWN_FIX (1<<9)
>  
>  struct i915_suspend_saved_registers {
>  	u32 saveDSPARB;

-- 
Jani Nikula, Intel Open Source Graphics Center

  reply	other threads:[~2022-12-12  9:15 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-10 18:01 [PATCH RFC] fix dell wyse 3040 poweroff Alexey Lukyanchuk
2022-12-10 18:01 ` [Intel-gfx] " Alexey Lukyanchuk
2022-12-10 18:01 ` Alexey Lukyanchuk
2022-12-12  9:15 ` Jani Nikula [this message]
2022-12-12  9:15   ` [Intel-gfx] " Jani Nikula
2022-12-12  9:15   ` Jani Nikula
2022-12-15 21:11 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87y1rdx9so.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=airlied@gmail.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rodrigo.vivi@intel.com \
    --cc=skif@skif-web.ru \
    --cc=tvrtko.ursulin@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.