| Message ID | 1606714572-1113-1-git-send-email-dikshita@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | venus: core: add support to dump FW region | expand |
On 11/30/20 7:36 AM, Dikshita Agarwal wrote: > Add support to dump video FW region during FW crash > using devcoredump helpers. > > Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org> > --- > drivers/media/platform/qcom/venus/core.c | 47 ++++++++++++++++++++++++++++++++ > 1 file changed, 47 insertions(+) > > diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c > index 6103aaf..01a0cfe 100644 > --- a/drivers/media/platform/qcom/venus/core.c > +++ b/drivers/media/platform/qcom/venus/core.c > @@ -7,8 +7,10 @@ > #include <linux/interconnect.h> > #include <linux/ioctl.h> > #include <linux/delay.h> > +#include <linux/devcoredump.h> > #include <linux/list.h> > #include <linux/module.h> > +#include <linux/of_address.h> > #include <linux/of_device.h> > #include <linux/platform_device.h> > #include <linux/slab.h> > @@ -22,6 +24,48 @@ > #include "firmware.h" > #include "pm_helpers.h" > > +static int subsystem_dump(struct venus_core *core) this function could return void because we don't handle the error in venus_sys_error_handler(). > +{ > + struct device_node *node; > + struct device *dev; > + struct resource r; > + void *mem_va; > + size_t mem_size; > + void *data; > + int ret; > + > + dev = core->dev; > + node = of_parse_phandle(dev->of_node, "memory-region", 0); > + if (!node) > + return -EINVAL; > + > + ret = of_address_to_resource(node, 0, &r); > + if (ret) > + goto err_put_node; > + > + mem_size = resource_size(&r); could you reuse the resource from venus_load_fw by adding r.sart and resource size in venus_core->fw{} structure. > + > + mem_va = memremap(r.start, mem_size, MEMREMAP_WC); > + if (!mem_va) { > + ret = -ENOMEM; > + goto err_put_node; > + } > + > + data = vmalloc(mem_size); > + if (!data) { > + ret = -EINVAL; > + goto err_unmap; > + } > + > + memcpy(data, mem_va, mem_size); > + > + dev_coredumpv(dev, data, mem_size, GFP_KERNEL); > +err_unmap: > + memunmap(mem_va); > +err_put_node: > + of_node_put(node); > + return ret; > +} > static void venus_event_notify(struct venus_core *core, u32 event) > { > struct venus_inst *inst; > @@ -67,6 +111,9 @@ static void venus_sys_error_handler(struct work_struct *work) > > venus_shutdown(core); > > + dev_warn(core->dev, "dumping FW region!\n"); this warn is not needed, please drop it. > + subsystem_dump(core); Are we sure that the dump is always required? > + > pm_runtime_put_sync(core->dev); > > while (core->pmdomains[0] && pm_runtime_active(core->pmdomains[0])) >
Quoting Dikshita Agarwal (2020-11-29 21:36:12) > diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c > index 6103aaf..01a0cfe 100644 > --- a/drivers/media/platform/qcom/venus/core.c > +++ b/drivers/media/platform/qcom/venus/core.c > @@ -22,6 +24,48 @@ > #include "firmware.h" > #include "pm_helpers.h" > > +static int subsystem_dump(struct venus_core *core) > +{ > + struct device_node *node; > + struct device *dev; > + struct resource r; > + void *mem_va; > + size_t mem_size; > + void *data; > + int ret; > + > + dev = core->dev; > + node = of_parse_phandle(dev->of_node, "memory-region", 0); Any chance this could be done at probe time and saved away as some sort of pointer? > + if (!node) > + return -EINVAL; > + > + ret = of_address_to_resource(node, 0, &r); of_node_put(node); > + if (ret) > + goto err_put_node; And then just return ret; > + > + mem_size = resource_size(&r); > + > + mem_va = memremap(r.start, mem_size, MEMREMAP_WC); > + if (!mem_va) { > + ret = -ENOMEM; > + goto err_put_node; And return -ENOMEM; > + } > + > + data = vmalloc(mem_size); > + if (!data) { > + ret = -EINVAL; > + goto err_unmap; > + } > + > + memcpy(data, mem_va, mem_size); > + > + dev_coredumpv(dev, data, mem_size, GFP_KERNEL); > +err_unmap: > + memunmap(mem_va); > +err_put_node: > + of_node_put(node); > + return ret; > +} > static void venus_event_notify(struct venus_core *core, u32 event) > { > struct venus_inst *inst; > @@ -67,6 +111,9 @@ static void venus_sys_error_handler(struct work_struct *work) > > venus_shutdown(core); > > + dev_warn(core->dev, "dumping FW region!\n"); Do we need this warning? > + subsystem_dump(core); Maybe call it venus_do_coredump() so it isn't so generic. > + > pm_runtime_put_sync(core->dev); > > while (core->pmdomains[0] && pm_runtime_active(core->pmdomains[0]))
diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c index 6103aaf..01a0cfe 100644 --- a/drivers/media/platform/qcom/venus/core.c +++ b/drivers/media/platform/qcom/venus/core.c @@ -7,8 +7,10 @@ #include <linux/interconnect.h> #include <linux/ioctl.h> #include <linux/delay.h> +#include <linux/devcoredump.h> #include <linux/list.h> #include <linux/module.h> +#include <linux/of_address.h> #include <linux/of_device.h> #include <linux/platform_device.h> #include <linux/slab.h> @@ -22,6 +24,48 @@ #include "firmware.h" #include "pm_helpers.h" +static int subsystem_dump(struct venus_core *core) +{ + struct device_node *node; + struct device *dev; + struct resource r; + void *mem_va; + size_t mem_size; + void *data; + int ret; + + dev = core->dev; + node = of_parse_phandle(dev->of_node, "memory-region", 0); + if (!node) + return -EINVAL; + + ret = of_address_to_resource(node, 0, &r); + if (ret) + goto err_put_node; + + mem_size = resource_size(&r); + + mem_va = memremap(r.start, mem_size, MEMREMAP_WC); + if (!mem_va) { + ret = -ENOMEM; + goto err_put_node; + } + + data = vmalloc(mem_size); + if (!data) { + ret = -EINVAL; + goto err_unmap; + } + + memcpy(data, mem_va, mem_size); + + dev_coredumpv(dev, data, mem_size, GFP_KERNEL); +err_unmap: + memunmap(mem_va); +err_put_node: + of_node_put(node); + return ret; +} static void venus_event_notify(struct venus_core *core, u32 event) { struct venus_inst *inst; @@ -67,6 +111,9 @@ static void venus_sys_error_handler(struct work_struct *work) venus_shutdown(core); + dev_warn(core->dev, "dumping FW region!\n"); + subsystem_dump(core); + pm_runtime_put_sync(core->dev); while (core->pmdomains[0] && pm_runtime_active(core->pmdomains[0]))
Add support to dump video FW region during FW crash using devcoredump helpers. Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org> --- drivers/media/platform/qcom/venus/core.c | 47 ++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)