[PATCH] drm/amdgpu: Remove wrapper layer of cgs irq handling
Zhang, Jerry (Junwei)
Jerry.Zhang at amd.com
Wed Mar 21 01:48:48 UTC 2018
On 03/20/2018 07:25 PM, Rex Zhu wrote:
> 1. remove struct cgs_os_ops
> 2. delete cgs_linux.h
> 3. refine the irq code for vega10
>
> Change-Id: I1b1b56c38596e632fe627c436a5072ae5b359b8c
> Signed-off-by: Rex Zhu <Rex.Zhu at amd.com>
Acked-by: Junwei Zhang <Jerry.Zhang at amd.com>
Nice to see that
Jerry
> ---
> drivers/gpu/drm/amd/acp/include/acp_gfx_if.h | 1 -
> drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c | 111 -------------------
> drivers/gpu/drm/amd/display/dc/os_types.h | 2 +-
> drivers/gpu/drm/amd/include/cgs_common.h | 1 -
> drivers/gpu/drm/amd/include/cgs_linux.h | 119 ---------------------
> drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c | 46 +-------
> drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c | 28 +++++
> drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.h | 5 +
> drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 36 ++++---
> drivers/gpu/drm/amd/powerplay/inc/hwmgr.h | 5 -
> 10 files changed, 56 insertions(+), 298 deletions(-)
> delete mode 100644 drivers/gpu/drm/amd/include/cgs_linux.h
>
> diff --git a/drivers/gpu/drm/amd/acp/include/acp_gfx_if.h b/drivers/gpu/drm/amd/acp/include/acp_gfx_if.h
> index a72ddb2f..feab8eb 100644
> --- a/drivers/gpu/drm/amd/acp/include/acp_gfx_if.h
> +++ b/drivers/gpu/drm/amd/acp/include/acp_gfx_if.h
> @@ -25,7 +25,6 @@
> #define _ACP_GFX_IF_H
>
> #include <linux/types.h>
> -#include "cgs_linux.h"
> #include "cgs_common.h"
>
> int amd_acp_hw_init(struct cgs_device *cgs_device,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
> index 5b37c1a..0f2b376 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
> @@ -28,7 +28,6 @@
> #include <linux/firmware.h>
> #include <drm/amdgpu_drm.h>
> #include "amdgpu.h"
> -#include "cgs_linux.h"
> #include "atom.h"
> #include "amdgpu_ucode.h"
>
> @@ -182,109 +181,6 @@ static int amdgpu_cgs_atom_exec_cmd_table(struct cgs_device *cgs_device, unsigne
> adev->mode_info.atom_context, table, args);
> }
>
> -struct cgs_irq_params {
> - unsigned src_id;
> - cgs_irq_source_set_func_t set;
> - cgs_irq_handler_func_t handler;
> - void *private_data;
> -};
> -
> -static int cgs_set_irq_state(struct amdgpu_device *adev,
> - struct amdgpu_irq_src *src,
> - unsigned type,
> - enum amdgpu_interrupt_state state)
> -{
> - struct cgs_irq_params *irq_params =
> - (struct cgs_irq_params *)src->data;
> - if (!irq_params)
> - return -EINVAL;
> - if (!irq_params->set)
> - return -EINVAL;
> - return irq_params->set(irq_params->private_data,
> - irq_params->src_id,
> - type,
> - (int)state);
> -}
> -
> -static int cgs_process_irq(struct amdgpu_device *adev,
> - struct amdgpu_irq_src *source,
> - struct amdgpu_iv_entry *entry)
> -{
> - struct cgs_irq_params *irq_params =
> - (struct cgs_irq_params *)source->data;
> - if (!irq_params)
> - return -EINVAL;
> - if (!irq_params->handler)
> - return -EINVAL;
> - return irq_params->handler(irq_params->private_data,
> - irq_params->src_id,
> - entry->iv_entry);
> -}
> -
> -static const struct amdgpu_irq_src_funcs cgs_irq_funcs = {
> - .set = cgs_set_irq_state,
> - .process = cgs_process_irq,
> -};
> -
> -static int amdgpu_cgs_add_irq_source(void *cgs_device,
> - unsigned client_id,
> - unsigned src_id,
> - unsigned num_types,
> - cgs_irq_source_set_func_t set,
> - cgs_irq_handler_func_t handler,
> - void *private_data)
> -{
> - CGS_FUNC_ADEV;
> - int ret = 0;
> - struct cgs_irq_params *irq_params;
> - struct amdgpu_irq_src *source =
> - kzalloc(sizeof(struct amdgpu_irq_src), GFP_KERNEL);
> - if (!source)
> - return -ENOMEM;
> - irq_params =
> - kzalloc(sizeof(struct cgs_irq_params), GFP_KERNEL);
> - if (!irq_params) {
> - kfree(source);
> - return -ENOMEM;
> - }
> - source->num_types = num_types;
> - source->funcs = &cgs_irq_funcs;
> - irq_params->src_id = src_id;
> - irq_params->set = set;
> - irq_params->handler = handler;
> - irq_params->private_data = private_data;
> - source->data = (void *)irq_params;
> - ret = amdgpu_irq_add_id(adev, client_id, src_id, source);
> - if (ret) {
> - kfree(irq_params);
> - kfree(source);
> - }
> -
> - return ret;
> -}
> -
> -static int amdgpu_cgs_irq_get(void *cgs_device, unsigned client_id,
> - unsigned src_id, unsigned type)
> -{
> - CGS_FUNC_ADEV;
> -
> - if (!adev->irq.client[client_id].sources)
> - return -EINVAL;
> -
> - return amdgpu_irq_get(adev, adev->irq.client[client_id].sources[src_id], type);
> -}
> -
> -static int amdgpu_cgs_irq_put(void *cgs_device, unsigned client_id,
> - unsigned src_id, unsigned type)
> -{
> - CGS_FUNC_ADEV;
> -
> - if (!adev->irq.client[client_id].sources)
> - return -EINVAL;
> -
> - return amdgpu_irq_put(adev, adev->irq.client[client_id].sources[src_id], type);
> -}
> -
> static int amdgpu_cgs_set_clockgating_state(struct cgs_device *cgs_device,
> enum amd_ip_block_type block_type,
> enum amd_clockgating_state state)
> @@ -792,12 +688,6 @@ static int amdgpu_cgs_notify_dpm_enabled(struct cgs_device *cgs_device, bool ena
> .lock_grbm_idx = amdgpu_cgs_lock_grbm_idx,
> };
>
> -static const struct cgs_os_ops amdgpu_cgs_os_ops = {
> - .add_irq_source = amdgpu_cgs_add_irq_source,
> - .irq_get = amdgpu_cgs_irq_get,
> - .irq_put = amdgpu_cgs_irq_put
> -};
> -
> struct cgs_device *amdgpu_cgs_create_device(struct amdgpu_device *adev)
> {
> struct amdgpu_cgs_device *cgs_device =
> @@ -809,7 +699,6 @@ struct cgs_device *amdgpu_cgs_create_device(struct amdgpu_device *adev)
> }
>
> cgs_device->base.ops = &amdgpu_cgs_ops;
> - cgs_device->base.os_ops = &amdgpu_cgs_os_ops;
> cgs_device->adev = adev;
>
> return (struct cgs_device *)cgs_device;
> diff --git a/drivers/gpu/drm/amd/display/dc/os_types.h b/drivers/gpu/drm/amd/display/dc/os_types.h
> index 1fcbc99..a407892 100644
> --- a/drivers/gpu/drm/amd/display/dc/os_types.h
> +++ b/drivers/gpu/drm/amd/display/dc/os_types.h
> @@ -32,7 +32,7 @@
>
> #include <linux/kref.h>
>
> -#include "cgs_linux.h"
> +#include "cgs_common.h"
>
> #if defined(__BIG_ENDIAN) && !defined(BIGENDIAN_CPU)
> #define BIGENDIAN_CPU
> diff --git a/drivers/gpu/drm/amd/include/cgs_common.h b/drivers/gpu/drm/amd/include/cgs_common.h
> index 6ff8f35..f2814ae 100644
> --- a/drivers/gpu/drm/amd/include/cgs_common.h
> +++ b/drivers/gpu/drm/amd/include/cgs_common.h
> @@ -290,7 +290,6 @@ struct cgs_ops {
> struct cgs_device
> {
> const struct cgs_ops *ops;
> - const struct cgs_os_ops *os_ops;
> /* to be embedded at the start of driver private structure */
> };
>
> diff --git a/drivers/gpu/drm/amd/include/cgs_linux.h b/drivers/gpu/drm/amd/include/cgs_linux.h
> deleted file mode 100644
> index bc7446c..0000000
> --- a/drivers/gpu/drm/amd/include/cgs_linux.h
> +++ /dev/null
> @@ -1,119 +0,0 @@
> -/*
> - * Copyright 2015 Advanced Micro Devices, Inc.
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a
> - * copy of this software and associated documentation files (the "Software"),
> - * to deal in the Software without restriction, including without limitation
> - * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> - * and/or sell copies of the Software, and to permit persons to whom the
> - * Software is furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice shall be included in
> - * all copies or substantial portions of the Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> - * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> - * OTHER DEALINGS IN THE SOFTWARE.
> - *
> - *
> - */
> -#ifndef _CGS_LINUX_H
> -#define _CGS_LINUX_H
> -
> -#include "cgs_common.h"
> -
> -/**
> - * cgs_irq_source_set_func() - Callback for enabling/disabling interrupt sources
> - * @private_data: private data provided to cgs_add_irq_source
> - * @src_id: interrupt source ID
> - * @type: interrupt type
> - * @enabled: 0 = disable source, non-0 = enable source
> - *
> - * Return: 0 on success, -errno otherwise
> - */
> -typedef int (*cgs_irq_source_set_func_t)(void *private_data,
> - unsigned src_id, unsigned type,
> - int enabled);
> -
> -/**
> - * cgs_irq_handler_func() - Interrupt handler callback
> - * @private_data: private data provided to cgs_add_irq_source
> - * @src_id: interrupt source ID
> - * @iv_entry: pointer to raw ih ring entry
> - *
> - * This callback runs in interrupt context.
> - *
> - * Return: 0 on success, -errno otherwise
> - */
> -typedef int (*cgs_irq_handler_func_t)(void *private_data,
> - unsigned src_id, const uint32_t *iv_entry);
> -
> -/**
> - * cgs_add_irq_source() - Add an IRQ source
> - * @cgs_device: opaque device handle
> - * @src_id: interrupt source ID
> - * @num_types: number of interrupt types that can be independently enabled
> - * @set: callback function to enable/disable an interrupt type
> - * @handler: interrupt handler callback
> - * @private_data: private data to pass to callback functions
> - *
> - * The same IRQ source can be added only once. Adding an IRQ source
> - * indicates ownership of that IRQ source and all its IRQ types.
> - *
> - * Return: 0 on success, -errno otherwise
> - */
> -typedef int (*cgs_add_irq_source_t)(void *cgs_device, unsigned client_id,
> - unsigned src_id,
> - unsigned num_types,
> - cgs_irq_source_set_func_t set,
> - cgs_irq_handler_func_t handler,
> - void *private_data);
> -
> -/**
> - * cgs_irq_get() - Request enabling an IRQ source and type
> - * @cgs_device: opaque device handle
> - * @src_id: interrupt source ID
> - * @type: interrupt type
> - *
> - * cgs_irq_get and cgs_irq_put calls must be balanced. They count
> - * "references" to IRQ sources.
> - *
> - * Return: 0 on success, -errno otherwise
> - */
> -typedef int (*cgs_irq_get_t)(void *cgs_device, unsigned client_id, unsigned src_id, unsigned type);
> -
> -/**
> - * cgs_irq_put() - Indicate IRQ source is no longer needed
> - * @cgs_device: opaque device handle
> - * @src_id: interrupt source ID
> - * @type: interrupt type
> - *
> - * cgs_irq_get and cgs_irq_put calls must be balanced. They count
> - * "references" to IRQ sources. Even after cgs_irq_put is called, the
> - * IRQ handler may still be called if there are more refecences to
> - * the IRQ source.
> - *
> - * Return: 0 on success, -errno otherwise
> - */
> -typedef int (*cgs_irq_put_t)(void *cgs_device, unsigned client_id, unsigned src_id, unsigned type);
> -
> -struct cgs_os_ops {
> - /* IRQ handling */
> - cgs_add_irq_source_t add_irq_source;
> - cgs_irq_get_t irq_get;
> - cgs_irq_put_t irq_put;
> -};
> -
> -#define cgs_add_irq_source(dev,client_id,src_id,num_types,set,handler,private_data) \
> - CGS_OS_CALL(add_irq_source,dev,client_id,src_id,num_types,set,handler, \
> - private_data)
> -#define cgs_irq_get(dev,client_id,src_id,type) \
> - CGS_OS_CALL(irq_get,dev,client_id,src_id,type)
> -#define cgs_irq_put(dev,client_id,src_id,type) \
> - CGS_OS_CALL(irq_put,dev,client_id,src_id,type)
> -
> -#endif /* _CGS_LINUX_H */
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> index 2290300..fbcdda5 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> @@ -56,50 +56,6 @@
> static int topaz_set_asic_special_caps(struct pp_hwmgr *hwmgr);
> static int ci_set_asic_special_caps(struct pp_hwmgr *hwmgr);
>
> -static int phm_thermal_l2h_irq(void *private_data,
> - unsigned src_id, const uint32_t *iv_entry)
> -{
> - struct pp_hwmgr *hwmgr = (struct pp_hwmgr *)private_data;
> - struct amdgpu_device *adev = hwmgr->adev;
> -
> - pr_warn("GPU over temperature range detected on PCIe %d:%d.%d!\n",
> - PCI_BUS_NUM(adev->pdev->devfn),
> - PCI_SLOT(adev->pdev->devfn),
> - PCI_FUNC(adev->pdev->devfn));
> - return 0;
> -}
> -
> -static int phm_thermal_h2l_irq(void *private_data,
> - unsigned src_id, const uint32_t *iv_entry)
> -{
> - struct pp_hwmgr *hwmgr = (struct pp_hwmgr *)private_data;
> - struct amdgpu_device *adev = hwmgr->adev;
> -
> - pr_warn("GPU under temperature range detected on PCIe %d:%d.%d!\n",
> - PCI_BUS_NUM(adev->pdev->devfn),
> - PCI_SLOT(adev->pdev->devfn),
> - PCI_FUNC(adev->pdev->devfn));
> - return 0;
> -}
> -
> -static int phm_ctf_irq(void *private_data,
> - unsigned src_id, const uint32_t *iv_entry)
> -{
> - struct pp_hwmgr *hwmgr = (struct pp_hwmgr *)private_data;
> - struct amdgpu_device *adev = hwmgr->adev;
> -
> - pr_warn("GPU Critical Temperature Fault detected on PCIe %d:%d.%d!\n",
> - PCI_BUS_NUM(adev->pdev->devfn),
> - PCI_SLOT(adev->pdev->devfn),
> - PCI_FUNC(adev->pdev->devfn));
> - return 0;
> -}
> -
> -static const struct cgs_irq_src_funcs thermal_irq_src[3] = {
> - { .handler = phm_thermal_l2h_irq },
> - { .handler = phm_thermal_h2l_irq },
> - { .handler = phm_ctf_irq }
> -};
>
> static void hwmgr_init_workload_prority(struct pp_hwmgr *hwmgr)
> {
> @@ -244,7 +200,7 @@ int hwmgr_hw_init(struct pp_hwmgr *hwmgr)
> if (ret)
> goto err2;
>
> - ret = phm_register_thermal_interrupt(hwmgr, &thermal_irq_src);
> + ret = phm_register_thermal_interrupt(hwmgr, NULL);
> if (ret)
> goto err2;
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c
> index e11daf5..dc1fe3b 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c
> @@ -534,3 +534,31 @@ int phm_get_voltage_evv_on_sclk(struct pp_hwmgr *hwmgr, uint8_t voltage_type,
> }
>
>
> +extern int phm_irq_process(struct amdgpu_device *adev,
> + struct amdgpu_irq_src *source,
> + struct amdgpu_iv_entry *entry)
> +{
> + uint32_t client_id = entry->client_id;
> + uint32_t src_id = entry->src_id;
> +
> + if (client_id == SOC15_IH_CLIENTID_THM) {
> + if (src_id == 0)
> + pr_warn("GPU over temperature range detected on PCIe %d:%d.%d!\n",
> + PCI_BUS_NUM(adev->pdev->devfn),
> + PCI_SLOT(adev->pdev->devfn),
> + PCI_FUNC(adev->pdev->devfn));
> + else
> + pr_warn("GPU under temperature range detected on PCIe %d:%d.%d!\n",
> + PCI_BUS_NUM(adev->pdev->devfn),
> + PCI_SLOT(adev->pdev->devfn),
> + PCI_FUNC(adev->pdev->devfn));
> + }
> +
> + if (client_id == SOC15_IH_CLIENTID_ROM_SMUIO)
> + pr_warn("GPU Critical Temperature Fault detected on PCIe %d:%d.%d!\n",
> + PCI_BUS_NUM(adev->pdev->devfn),
> + PCI_SLOT(adev->pdev->devfn),
> + PCI_FUNC(adev->pdev->devfn));
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.h b/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.h
> index a1a4913..8e84760 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.h
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.h
> @@ -73,6 +73,11 @@ extern int phm_wait_on_indirect_register(struct pp_hwmgr *hwmgr,
> uint32_t value,
> uint32_t mask);
>
> +extern int phm_irq_process(struct amdgpu_device *adev,
> + struct amdgpu_irq_src *source,
> + struct amdgpu_iv_entry *entry);
> +
> +
> #define PHM_FIELD_SHIFT(reg, field) reg##__##field##__SHIFT
> #define PHM_FIELD_MASK(reg, field) reg##__##field##_MASK
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> index 5521137..f9c9f42 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> @@ -44,7 +44,6 @@
> #include "vega10_thermal.h"
> #include "pp_debug.h"
> #include "amd_pcie_helpers.h"
> -#include "cgs_linux.h"
> #include "ppinterrupt.h"
> #include "pp_overdriver.h"
> #include "pp_thermal.h"
> @@ -4816,34 +4815,41 @@ static int vega10_get_thermal_temperature_range(struct pp_hwmgr *hwmgr,
> return 0;
> }
>
> +static const struct amdgpu_irq_src_funcs smu9_irq_funcs = {
> + .process = phm_irq_process,
> +};
> +
> static int vega10_register_thermal_interrupt(struct pp_hwmgr *hwmgr,
> const void *info)
> {
> - struct cgs_irq_src_funcs *irq_src =
> - (struct cgs_irq_src_funcs *)info;
> + struct amdgpu_irq_src *source =
> + kzalloc(sizeof(struct amdgpu_irq_src), GFP_KERNEL);
> +
> + if (!source)
> + return -ENOMEM;
> +
> + source->funcs = &smu9_irq_funcs;
>
> if (hwmgr->thermal_controller.ucType ==
> ATOM_VEGA10_PP_THERMALCONTROLLER_VEGA10 ||
> hwmgr->thermal_controller.ucType ==
> ATOM_VEGA10_PP_THERMALCONTROLLER_EMC2103_WITH_INTERNAL) {
> - PP_ASSERT_WITH_CODE(!cgs_add_irq_source(hwmgr->device,
> +
> + amdgpu_irq_add_id((struct amdgpu_device *)(hwmgr->adev),
> SOC15_IH_CLIENTID_THM,
> - 0, 0, irq_src[0].set, irq_src[0].handler, hwmgr),
> - "Failed to register high thermal interrupt!",
> - return -EINVAL);
> - PP_ASSERT_WITH_CODE(!cgs_add_irq_source(hwmgr->device,
> + 0,
> + source);
> + amdgpu_irq_add_id((struct amdgpu_device *)(hwmgr->adev),
> SOC15_IH_CLIENTID_THM,
> - 1, 0, irq_src[1].set, irq_src[1].handler, hwmgr),
> - "Failed to register low thermal interrupt!",
> - return -EINVAL);
> + 1,
> + source);
> }
>
> /* Register CTF(GPIO_19) interrupt */
> - PP_ASSERT_WITH_CODE(!cgs_add_irq_source(hwmgr->device,
> + amdgpu_irq_add_id((struct amdgpu_device *)(hwmgr->adev),
> SOC15_IH_CLIENTID_ROM_SMUIO,
> - 83, 0, irq_src[2].set, irq_src[2].handler, hwmgr),
> - "Failed to register CTF thermal interrupt!",
> - return -EINVAL);
> + 83,
> + source);
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> index 85b46ad..2667775 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> @@ -764,11 +764,6 @@ struct pp_hwmgr {
> uint32_t workload_setting[Workload_Policy_Max];
> };
>
> -struct cgs_irq_src_funcs {
> - cgs_irq_source_set_func_t set;
> - cgs_irq_handler_func_t handler;
> -};
> -
> extern int hwmgr_early_init(struct pp_hwmgr *hwmgr);
> extern int hwmgr_hw_init(struct pp_hwmgr *hwmgr);
> extern int hwmgr_hw_fini(struct pp_hwmgr *hwmgr);
>
More information about the amd-gfx
mailing list