AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Christian König" <ckoenig.leichtzumerken@gmail.com>
To: Wenhui Sheng <Wenhui.Sheng@amd.com>, amd-gfx@lists.freedesktop.org
Cc: Likun Gao <Likun.Gao@amd.com>, Hawking Zhang <Hawking.Zhang@amd.com>
Subject: Re: [PATCH 4/4] drm/amdgpu: add module parameter choose reset mode
Date: Tue, 14 Jul 2020 10:35:45 +0200	[thread overview]
Message-ID: <fc1653c7-1e3e-91fe-1692-e66ce1be59c3@gmail.com> (raw)
In-Reply-To: <20200714022911.3006-4-Wenhui.Sheng@amd.com>

Am 14.07.20 um 04:29 schrieb Wenhui Sheng:
> Default value is auto, doesn't change
> original reset method logic.
>
> v2: change to use parameter reset_method
> v3: add warn msg if specified mode isn't supported
>
> Signed-off-by: Likun Gao <Likun.Gao@amd.com>
> Signed-off-by: Wenhui Sheng <Wenhui.Sheng@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h     | 1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8 ++++++++
>   drivers/gpu/drm/amd/amdgpu/cik.c        | 7 +++++++
>   drivers/gpu/drm/amd/amdgpu/nv.c         | 7 +++++++
>   drivers/gpu/drm/amd/amdgpu/si.c         | 5 +++++
>   drivers/gpu/drm/amd/amdgpu/soc15.c      | 8 ++++++++
>   drivers/gpu/drm/amd/amdgpu/vi.c         | 7 +++++++
>   7 files changed, 43 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 4de93cef79b9..06bfb8658dec 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -196,6 +196,7 @@ static const bool debug_evictions; /* = false */
>   #endif
>   
>   extern int amdgpu_tmz;
> +extern int amdgpu_reset_method;
>   
>   #ifdef CONFIG_DRM_AMDGPU_SI
>   extern int amdgpu_si_support;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 94c83a9d4987..581d5fcac361 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -154,6 +154,7 @@ int amdgpu_mes = 0;
>   int amdgpu_noretry = 1;
>   int amdgpu_force_asic_type = -1;
>   int amdgpu_tmz = 0;
> +int amdgpu_reset_method = -1; /* auto */
>   
>   struct amdgpu_mgpu_info mgpu_info = {
>   	.mutex = __MUTEX_INITIALIZER(mgpu_info.mutex),
> @@ -793,6 +794,13 @@ module_param_named(abmlevel, amdgpu_dm_abm_level, uint, 0444);
>   MODULE_PARM_DESC(tmz, "Enable TMZ feature (-1 = auto, 0 = off (default), 1 = on)");
>   module_param_named(tmz, amdgpu_tmz, int, 0444);
>   
> +/**
> + * DOC: reset_method (int)
> + * GPU reset method (-1 = auto (default), 0 = legacy, 1 = mode0, 2 = mode1, 3 = mode2, 4 = baco)
> + */
> +MODULE_PARM_DESC(reset_method, "GPU reset method (-1 = auto (default), 0 = legacy, 1 = mode0, 2 = mode1, 3 = mode2, 4 = baco)");
> +module_param_named(reset_method, amdgpu_reset_method, int, 0444);
> +
>   static const struct pci_device_id pciidlist[] = {
>   #ifdef  CONFIG_DRM_AMDGPU_SI
>   	{0x1002, 0x6780, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_TAHITI},
> diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c
> index fe306d0f73f7..310bcf81256f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/cik.c
> +++ b/drivers/gpu/drm/amd/amdgpu/cik.c
> @@ -1326,6 +1326,13 @@ cik_asic_reset_method(struct amdgpu_device *adev)
>   {
>   	bool baco_reset;
>   
> +	if (amdgpu_reset_method == AMD_RESET_METHOD_LEGACY ||
> +	    amdgpu_reset_method == AMD_RESET_METHOD_BACO)
> +		return amdgpu_reset_method;
> +	else if (amdgpu_reset_method != -1)

When you return anyway you can also drop the else here and on other 
occasions as well.

Apart from that the patch looks good to me.

We usually try to avoid adding more module parameters, but I think this 
one is really justified.

Thanks,
Christian.

> +		dev_warn(adev->dev, "Specified reset:%d isn't supported, using AUTO instead.\n",
> +				  amdgpu_reset_method);
> +
>   	switch (adev->asic_type) {
>   	case CHIP_BONAIRE:
>   	case CHIP_HAWAII:
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
> index 9f1240bd0310..486321bcab8f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> @@ -311,6 +311,13 @@ nv_asic_reset_method(struct amdgpu_device *adev)
>   {
>   	struct smu_context *smu = &adev->smu;
>   
> +	if (amdgpu_reset_method == AMD_RESET_METHOD_MODE1 ||
> +	    amdgpu_reset_method == AMD_RESET_METHOD_BACO)
> +		return amdgpu_reset_method;
> +	else if (amdgpu_reset_method != -1)
> +		dev_warn(adev->dev, "Specified reset method:%d isn't supported, using AUTO instead.\n",
> +				  amdgpu_reset_method);
> +
>   	if (smu_baco_is_support(smu))
>   		return AMD_RESET_METHOD_BACO;
>   	else
> diff --git a/drivers/gpu/drm/amd/amdgpu/si.c b/drivers/gpu/drm/amd/amdgpu/si.c
> index 9d7b4ccd17b8..1b449291f068 100644
> --- a/drivers/gpu/drm/amd/amdgpu/si.c
> +++ b/drivers/gpu/drm/amd/amdgpu/si.c
> @@ -1229,6 +1229,11 @@ static bool si_asic_supports_baco(struct amdgpu_device *adev)
>   static enum amd_reset_method
>   si_asic_reset_method(struct amdgpu_device *adev)
>   {
> +	if (amdgpu_reset_method != AMD_RESET_METHOD_LEGACY &&
> +	    amdgpu_reset_method != -1)
> +		dev_warn(adev->dev, "Specified reset method:%d isn't supported, using AUTO instead.\n",
> +				  amdgpu_reset_method);
> +
>   	return AMD_RESET_METHOD_LEGACY;
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
> index 8c739b285915..40b343b25588 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> @@ -532,6 +532,14 @@ soc15_asic_reset_method(struct amdgpu_device *adev)
>   	bool baco_reset = false;
>   	struct amdgpu_ras *ras = amdgpu_ras_get_context(adev);
>   
> +	if (amdgpu_reset_method == AMD_RESET_METHOD_MODE1 ||
> +	    amdgpu_reset_method == AMD_RESET_METHOD_MODE2 ||
> +		amdgpu_reset_method == AMD_RESET_METHOD_BACO)
> +		return amdgpu_reset_method;
> +	else if (amdgpu_reset_method != -1)
> +		dev_warn(adev->dev, "Specified reset method:%d isn't supported, using AUTO instead.\n",
> +				  amdgpu_reset_method);
> +
>   	switch (adev->asic_type) {
>   	case CHIP_RAVEN:
>   	case CHIP_RENOIR:
> diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c
> index 4e5e91888d87..e4628c17802f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vi.c
> @@ -710,6 +710,13 @@ vi_asic_reset_method(struct amdgpu_device *adev)
>   {
>   	bool baco_reset;
>   
> +	if (amdgpu_reset_method == AMD_RESET_METHOD_LEGACY ||
> +	    amdgpu_reset_method == AMD_RESET_METHOD_BACO)
> +		return amdgpu_reset_method;
> +	else if (amdgpu_reset_method != -1)
> +		dev_warn(adev->dev, "Specified reset method:%d isn't supported, using AUTO instead.\n",
> +				  amdgpu_reset_method);
> +
>   	switch (adev->asic_type) {
>   	case CHIP_FIJI:
>   	case CHIP_TONGA:

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

  parent reply	other threads:[~2020-07-14  8:35 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-14  2:29 [PATCH 1/4] drm/amd/powerplay: add SMU mode1 reset Wenhui Sheng
2020-07-14  2:29 ` [PATCH 2/4] drm/amdgpu: RAS emergency restart logic refine Wenhui Sheng
2020-07-14  2:29 ` [PATCH 3/4] drm/amdgpu: enable mode1 reset Wenhui Sheng
2020-07-14  2:29 ` [PATCH 4/4] drm/amdgpu: add module parameter choose reset mode Wenhui Sheng
2020-07-14  6:39   ` Zhang, Hawking
2020-07-14  8:35   ` Christian König [this message]
2020-07-14  9:58     ` Sheng, Wenhui
2020-07-14 11:45       ` Christian König
2020-07-15  3:06         ` Sheng, Wenhui
2020-07-15  9:08           ` Christian König

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=fc1653c7-1e3e-91fe-1692-e66ce1be59c3@gmail.com \
    --to=ckoenig.leichtzumerken@gmail.com \
    --cc=Hawking.Zhang@amd.com \
    --cc=Likun.Gao@amd.com \
    --cc=Wenhui.Sheng@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=christian.koenig@amd.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox