[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fdbdbf61-e372-b81d-3a14-7ed27b1249a3@linaro.org>
Date: Fri, 4 Aug 2023 21:51:22 +0100
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Konrad Dybcio <konrad.dybcio@...aro.org>,
Stanimir Varbanov <stanimir.k.varbanov@...il.com>,
Vikash Garodia <quic_vgarodia@...cinc.com>,
Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
Andy Gross <agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>
Cc: Marijn Suijten <marijn.suijten@...ainline.org>,
Konrad Dybcio <konradybcio@...nel.org>,
linux-media@...r.kernel.org, linux-arm-msm@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/6] media: venus: hfi_venus: Support only updating
certain bits with presets
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@...aro.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@...aro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
Powered by blists - more mailing lists