| Message ID | 20230731-topic-8280_venus-v1-3-8c8bbe1983a5@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | SM8350 and SC8280XP venus support | expand |
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>
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>
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; + + for (i = 0; i < count; i++) { + val = tbl[i].value; - 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 */ + if (tbl[i].mask) { + val = readl(hdev->core->base + tbl[i].reg); + val = (val & ~tbl[i].mask) | (tbl[i].value & tbl[i].mask); + } + + writel(val, hdev->core->base + tbl[i].reg); + } } static void venus_soft_int(struct venus_hfi_device *hdev)
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(-)