Messages in this thread | | | Date | Fri, 4 Aug 2023 21:51:22 +0100 | Subject | Re: [PATCH 3/6] media: venus: hfi_venus: Support only updating certain bits with presets | From | Bryan O'Donoghue <> |
| |
On 04/08/2023 21:50, Bryan O'Donoghue wrote: > On 04/08/2023 21:09, Konrad Dybcio wrote: >> On some platforms (like SM8350) we're expected to only touch certain bits >> (such as 0 and 4 corresponding to mask 0x11). Add support for doing so. >> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >> --- >> drivers/media/platform/qcom/venus/core.h | 1 + >> drivers/media/platform/qcom/venus/hfi_venus.c | 15 ++++++++++++--- >> 2 files changed, 13 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/media/platform/qcom/venus/core.h >> b/drivers/media/platform/qcom/venus/core.h >> index d996abd339e1..2999c24392f5 100644 >> --- a/drivers/media/platform/qcom/venus/core.h >> +++ b/drivers/media/platform/qcom/venus/core.h >> @@ -38,6 +38,7 @@ struct freq_tbl { >> struct reg_val { >> u32 reg; >> u32 value; >> + u32 mask; >> }; >> struct bw_tbl { >> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c >> b/drivers/media/platform/qcom/venus/hfi_venus.c >> index 19fc6575a489..d4bad66f7293 100644 >> --- a/drivers/media/platform/qcom/venus/hfi_venus.c >> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c >> @@ -349,10 +349,19 @@ static void venus_set_registers(struct >> venus_hfi_device *hdev) >> const struct venus_resources *res = hdev->core->res; >> const struct reg_val *tbl = res->reg_tbl; >> unsigned int count = res->reg_tbl_size; >> - unsigned int i; >> + unsigned int i, val; > > u32 val; > >> + >> + for (i = 0; i < count; i++) { >> + val = tbl[i].value; > > struct reg_val looks like this > > struct reg_val { > u32 reg; > u32 value; > }; > > val should be declared a u32 > >> - for (i = 0; i < count; i++) >> - writel(tbl[i].value, hdev->core->base + tbl[i].reg); >> + /* In some cases, we only want to update certain bits */ > > I'll trust this is a legitimate and true statement. > >> + if (tbl[i].mask) { >> + val = readl(hdev->core->base + tbl[i].reg); >> + val = (val & ~tbl[i].mask) | (tbl[i].value & tbl[i].mask); > > feels like something regmap_update_bits() already does though, I prefer > this because there's less code in it. > >> + } >> + >> + writel(val, hdev->core->base + tbl[i].reg); >> + } > > With the val declaration fix > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
| |