Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

media: amphion: fix a bug that vpu core may not resume after suspend

driver will enable the vpu core when request the first instance
on the core.
one vpu core can only support 8 streaming instances in the same
time, the instance won't be added to core's list before streamon.

so the actual instance count may be greater then the number in
the core's list.

in pm resume callback, driver will resume the core immediately if
core's list is not empty.
but this check is not accurate,
if suspend during one instance is requested, but not streamon,
then after suspend, the core won't be resume, and led to instance failure.

use the request_count instead of the core's list to check
whether is the core needed to resume immediately after suspend.

And it can make the pm suspend and resume callback more clear.

Fixes: 9f599f351e86 ("media: amphion: add vpu core driver")
Signed-off-by: Ming Qian <ming.qian@nxp.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

authored by

Ming Qian and committed by
Mauro Carvalho Chehab
0202a665 c65c3f3a

+51 -44
-1
drivers/media/platform/amphion/vpu.h
··· 119 119 enum vpu_core_state { 120 120 VPU_CORE_DEINIT = 0, 121 121 VPU_CORE_ACTIVE, 122 - VPU_CORE_SNAPSHOT, 123 122 VPU_CORE_HANG 124 123 }; 125 124
+43 -41
drivers/media/platform/amphion/vpu_core.c
··· 89 89 core->supported_instance_count = min(core->supported_instance_count, count); 90 90 } 91 91 core->fw_version = fw_version; 92 - core->state = VPU_CORE_ACTIVE; 92 + vpu_core_set_state(core, VPU_CORE_ACTIVE); 93 93 94 94 return 0; 95 95 } ··· 172 172 return __vpu_alloc_dma(core->dev, buf); 173 173 } 174 174 175 - static void vpu_core_check_hang(struct vpu_core *core) 175 + void vpu_core_set_state(struct vpu_core *core, enum vpu_core_state state) 176 176 { 177 - if (core->hang_mask) 178 - core->state = VPU_CORE_HANG; 177 + if (state != core->state) 178 + vpu_trace(core->dev, "vpu core state change from %d to %d\n", core->state, state); 179 + core->state = state; 180 + if (core->state == VPU_CORE_DEINIT) 181 + core->hang_mask = 0; 182 + } 183 + 184 + static void vpu_core_update_state(struct vpu_core *core) 185 + { 186 + if (!vpu_iface_get_power_state(core)) { 187 + if (core->request_count) 188 + vpu_core_set_state(core, VPU_CORE_HANG); 189 + else 190 + vpu_core_set_state(core, VPU_CORE_DEINIT); 191 + 192 + } else if (core->state == VPU_CORE_ACTIVE && core->hang_mask) { 193 + vpu_core_set_state(core, VPU_CORE_HANG); 194 + } 179 195 } 180 196 181 197 static struct vpu_core *vpu_core_find_proper_by_type(struct vpu_dev *vpu, u32 type) ··· 204 188 dev_dbg(c->dev, "instance_mask = 0x%lx, state = %d\n", c->instance_mask, c->state); 205 189 if (c->type != type) 206 190 continue; 191 + mutex_lock(&c->lock); 192 + vpu_core_update_state(c); 193 + mutex_unlock(&c->lock); 207 194 if (c->state == VPU_CORE_DEINIT) { 208 195 core = c; 209 196 break; 210 197 } 211 - vpu_core_check_hang(c); 212 198 if (c->state != VPU_CORE_ACTIVE) 213 199 continue; 214 200 if (c->request_count < request_count) { ··· 427 409 } 428 410 429 411 mutex_lock(&core->lock); 412 + if (core->state != VPU_CORE_ACTIVE) { 413 + dev_err(core->dev, "vpu core is not active, state = %d\n", core->state); 414 + ret = -EINVAL; 415 + goto exit; 416 + } 417 + 430 418 if (inst->id >= 0 && inst->id < core->supported_instance_count) 431 419 goto exit; 432 420 ··· 474 450 vpu_core_release_instance(core, inst->id); 475 451 inst->id = VPU_INST_NULL_ID; 476 452 } 477 - vpu_core_check_hang(core); 453 + vpu_core_update_state(core); 478 454 if (core->state == VPU_CORE_HANG && !core->instance_mask) { 479 455 int err; 480 456 ··· 483 459 err = vpu_core_sw_reset(core); 484 460 mutex_lock(&core->lock); 485 461 if (!err) { 486 - core->state = VPU_CORE_ACTIVE; 462 + vpu_core_set_state(core, VPU_CORE_ACTIVE); 487 463 core->hang_mask = 0; 488 464 } 489 465 } ··· 633 609 mutex_init(&core->cmd_lock); 634 610 init_completion(&core->cmp); 635 611 init_waitqueue_head(&core->ack_wq); 636 - core->state = VPU_CORE_DEINIT; 612 + vpu_core_set_state(core, VPU_CORE_DEINIT); 637 613 638 614 core->res = of_device_get_match_data(dev); 639 615 if (!core->res) ··· 782 758 mutex_lock(&core->lock); 783 759 pm_runtime_resume_and_get(dev); 784 760 vpu_core_get_vpu(core); 785 - if (core->state != VPU_CORE_SNAPSHOT) 786 - goto exit; 787 761 788 - if (!vpu_iface_get_power_state(core)) { 789 - if (!list_empty(&core->instances)) { 762 + if (core->request_count) { 763 + if (!vpu_iface_get_power_state(core)) 790 764 ret = vpu_core_boot(core, false); 791 - if (ret) { 792 - dev_err(core->dev, "%s boot fail\n", __func__); 793 - core->state = VPU_CORE_DEINIT; 794 - goto exit; 795 - } 796 - } else { 797 - core->state = VPU_CORE_DEINIT; 798 - } 799 - } else { 800 - if (!list_empty(&core->instances)) { 765 + else 801 766 ret = vpu_core_sw_reset(core); 802 - if (ret) { 803 - dev_err(core->dev, "%s sw_reset fail\n", __func__); 804 - core->state = VPU_CORE_HANG; 805 - goto exit; 806 - } 767 + if (ret) { 768 + dev_err(core->dev, "resume fail\n"); 769 + vpu_core_set_state(core, VPU_CORE_HANG); 807 770 } 808 - core->state = VPU_CORE_ACTIVE; 809 771 } 810 - 811 - exit: 772 + vpu_core_update_state(core); 812 773 pm_runtime_put_sync(dev); 813 774 mutex_unlock(&core->lock); 814 775 ··· 807 798 int ret = 0; 808 799 809 800 mutex_lock(&core->lock); 810 - if (core->state == VPU_CORE_ACTIVE) { 811 - if (!list_empty(&core->instances)) { 812 - ret = vpu_core_snapshot(core); 813 - if (ret) { 814 - mutex_unlock(&core->lock); 815 - return ret; 816 - } 817 - } 818 - 819 - core->state = VPU_CORE_SNAPSHOT; 820 - } 801 + if (core->request_count) 802 + ret = vpu_core_snapshot(core); 821 803 mutex_unlock(&core->lock); 804 + if (ret) 805 + return ret; 822 806 823 807 vpu_core_cancel_work(core); 824 808
+1
drivers/media/platform/amphion/vpu_core.h
··· 11 11 int vpu_alloc_dma(struct vpu_core *core, struct vpu_buffer *buf); 12 12 void vpu_free_dma(struct vpu_buffer *buf); 13 13 struct vpu_inst *vpu_core_find_instance(struct vpu_core *core, u32 index); 14 + void vpu_core_set_state(struct vpu_core *core, enum vpu_core_state state); 14 15 15 16 #endif
+7 -2
drivers/media/platform/amphion/vpu_dbg.c
··· 15 15 #include <linux/debugfs.h> 16 16 #include "vpu.h" 17 17 #include "vpu_defs.h" 18 + #include "vpu_core.h" 18 19 #include "vpu_helpers.h" 19 20 #include "vpu_cmds.h" 20 21 #include "vpu_rpc.h" ··· 234 233 if (seq_write(s, str, num)) 235 234 return 0; 236 235 236 + num = scnprintf(str, sizeof(str), "power %s\n", 237 + vpu_iface_get_power_state(core) ? "on" : "off"); 238 + if (seq_write(s, str, num)) 239 + return 0; 237 240 num = scnprintf(str, sizeof(str), "state = %d\n", core->state); 238 241 if (seq_write(s, str, num)) 239 242 return 0; ··· 351 346 352 347 pm_runtime_resume_and_get(core->dev); 353 348 mutex_lock(&core->lock); 354 - if (core->state != VPU_CORE_DEINIT && !core->instance_mask) { 349 + if (vpu_iface_get_power_state(core) && !core->request_count) { 355 350 dev_info(core->dev, "reset\n"); 356 351 if (!vpu_core_sw_reset(core)) { 357 - core->state = VPU_CORE_ACTIVE; 352 + vpu_core_set_state(core, VPU_CORE_ACTIVE); 358 353 core->hang_mask = 0; 359 354 } 360 355 }