[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAFr9PX=6Pd1Rg=wJvpuX6WX63L=iAnwPA24e59An3Kac5f_vzA@mail.gmail.com>
Date: Sat, 4 Dec 2021 22:43:44 +0900
From: Daniel Palmer <daniel@...f.com>
To: Dafna Hirschfeld <dafna.hirschfeld@...labora.com>
Cc: "open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)"
<linux-media@...r.kernel.org>,
Robert Beckett <bob.beckett@...labora.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"open list:STAGING SUBSYSTEM" <linux-staging@...ts.linux.dev>,
open list <linux-kernel@...r.kernel.org>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
hverkuil@...all.nl, kernel@...labora.com, dafna3@...il.com,
kiril.bicevski@...labora.com,
Nas Chung <nas.chung@...psnmedia.com>,
lafley.kim@...psnmedia.com, scott.woo@...psnmedia.com,
olivier.crete@...labora.com, dan.carpenter@...cle.com,
Randy Dunlap <rdunlap@...radead.org>
Subject: Re: [PATCH v4 1/6] staging: media: wave5: Add vpuapi layer
Hi Dafna,
Sorry for the piecemeal emails..
On Thu, 2 Dec 2021 at 02:56, Dafna Hirschfeld
<dafna.hirschfeld@...labora.com> wrote:
> diff --git a/drivers/staging/media/wave5/wave5-hw.c b/drivers/staging/media/wave5/wave5-hw.c
... snip ...
> +static int wave5_wait_bus_busy(struct vpu_device *vpu_dev, int timeout, unsigned int addr)
> +{
> + u32 gdi_status_check_value = 0x3f;
> + u32 data;
> +
> + if (vpu_dev->product_code == WAVE521C_CODE ||
> + vpu_dev->product_code == WAVE521_CODE ||
> + vpu_dev->product_code == WAVE521E1_CODE)
> + gdi_status_check_value = 0x00ff1f3f;
> +
> + return read_poll_timeout(wave5_vdi_read_register, data, data == gdi_status_check_value,
> + 0, timeout * 1000, false, vpu_dev, addr);
> +}
> +
This looks like it should be s/wave5_vdi_read_register/wave5_read_register/.
For wave511 addr passed in here is 0x8e14 so well outside of what is
directly accessible.
Also it seems that this can either return 0 or -ETIMEDOUT...
... snip ...
> +int wave5_vpu_reset(struct device *dev, enum sw_reset_mode reset_mode)
> +{
> + u32 val = 0;
> + int ret = 0;
> + struct vpu_device *vpu_dev = dev_get_drvdata(dev);
> + struct vpu_attr *p_attr = &vpu_dev->attr;
> + // VPU doesn't send response. force to set BUSY flag to 0.
> + vpu_write_reg(vpu_dev, W5_VPU_BUSY_STATUS, 0);
> +
> + if (reset_mode == SW_RESET_SAFETY) {
> + ret = wave5_vpu_sleep_wake(dev, true, NULL, 0);
> + if (ret)
> + return ret;
> + }
> +
> + val = vpu_read_reg(vpu_dev, W5_VPU_RET_VPU_CONFIG0);
> + if ((val >> 16) & 0x1)
> + p_attr->support_backbone = true;
> + if ((val >> 22) & 0x1)
> + p_attr->support_vcore_backbone = true;
> + if ((val >> 28) & 0x1)
> + p_attr->support_vcpu_backbone = true;
> +
> + val = vpu_read_reg(vpu_dev, W5_VPU_RET_VPU_CONFIG1);
> + if ((val >> 26) & 0x1)
> + p_attr->support_dual_core = true;
> +
> + // waiting for completion of bus transaction
> + if (p_attr->support_backbone) {
> + if (p_attr->support_dual_core) {
> + // check CORE0
> + wave5_write_register(vpu_dev, W5_BACKBONE_BUS_CTRL_VCORE0, 0x7);
> +
> + ret = wave5_wait_bus_busy(vpu_dev, VPU_BUSY_CHECK_TIMEOUT,
> + W5_BACKBONE_BUS_STATUS_VCORE0);
> + if (ret) {
> + wave5_write_register(vpu_dev, W5_BACKBONE_BUS_CTRL_VCORE0, 0x00);
> + return ret;
> + }
> +
> + // check CORE1
> + wave5_write_register(vpu_dev, W5_BACKBONE_BUS_CTRL_VCORE1, 0x7);
> +
> + ret = wave5_wait_bus_busy(vpu_dev, VPU_BUSY_CHECK_TIMEOUT,
> + W5_BACKBONE_BUS_STATUS_VCORE1);
> + if (ret) {
> + wave5_write_register(vpu_dev, W5_BACKBONE_BUS_CTRL_VCORE1, 0x00);
> + return ret;
> + }
> +
> + } else if (p_attr->support_vcore_backbone) {
> + if (p_attr->support_vcpu_backbone) {
> + // step1 : disable request
> + wave5_write_register(vpu_dev, W5_BACKBONE_BUS_CTRL_VCPU,
> + 0xFF);
> +
> + // step2 : waiting for completion of bus transaction
> + ret = wave5_wait_vcpu_bus_busy(vpu_dev, VPU_BUSY_CHECK_TIMEOUT,
> + W5_BACKBONE_BUS_STATUS_VCPU);
> + if (ret) {
> + wave5_write_register(vpu_dev,
> + W5_BACKBONE_BUS_CTRL_VCPU, 0x00);
> + return ret;
> + }
> + }
> + // step1 : disable request
> + wave5_write_register(vpu_dev, W5_BACKBONE_BUS_CTRL_VCORE0, 0x7);
> +
> + // step2 : waiting for completion of bus transaction
> + if (wave5_wait_bus_busy(vpu_dev, VPU_BUSY_CHECK_TIMEOUT,
> + W5_BACKBONE_BUS_STATUS_VCORE0) == -1) {
> + wave5_write_register(vpu_dev, W5_BACKBONE_BUS_CTRL_VCORE0, 0x00);
> + return -EBUSY;
> + }
but this is looking for -1 on failure.
> + } else {
> + // step1 : disable request
> + wave5_write_register(vpu_dev, W5_COMBINED_BACKBONE_BUS_CTRL, 0x7);
> +
> + // step2 : waiting for completion of bus transaction
> + if (wave5_wait_bus_busy(vpu_dev, VPU_BUSY_CHECK_TIMEOUT,
> + W5_COMBINED_BACKBONE_BUS_STATUS) == -1) {
> + wave5_write_register(vpu_dev, W5_COMBINED_BACKBONE_BUS_CTRL, 0x00);
> + return -EBUSY;
> + }
> + }
Here too.
Cheers,
Daniel
Powered by blists - more mailing lists