Messages in this thread | | | Date | Thu, 11 Jan 2024 23:39:36 +0530 | Subject | Re: [PATCH v7 3/5] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings | From | Bibek Kumar Patro <> |
| |
On 1/10/2024 11:26 PM, Konrad Dybcio wrote: > > > On 1/10/24 13:55, Bibek Kumar Patro wrote: >> >> >> On 1/10/2024 4:46 PM, Bibek Kumar Patro wrote: >>> >>> >>> On 1/10/2024 9:36 AM, Pavan Kondeti wrote: >> >> [...] >> >>>>> @@ -274,6 +321,21 @@ static const struct of_device_id >>>>> qcom_smmu_client_of_match[] __maybe_unused = { >>>>> static int qcom_smmu_init_context(struct arm_smmu_domain >>>>> *smmu_domain, >>>>> struct io_pgtable_cfg *pgtbl_cfg, struct device *dev) >>>>> { >>>>> + struct arm_smmu_device *smmu = smmu_domain->smmu; >>>>> + struct qcom_smmu *qsmmu = to_qcom_smmu(smmu); >>>>> + const struct actlr_variant *actlrvar; >>>>> + int cbndx = smmu_domain->cfg.cbndx; >>>>> + >>>>> + if (qsmmu->data->actlrvar) { >>>>> + actlrvar = qsmmu->data->actlrvar; >>>>> + for (; actlrvar->io_start; actlrvar++) { >>>>> + if (actlrvar->io_start == smmu->ioaddr) { >>>>> + qcom_smmu_set_actlr(dev, smmu, cbndx, >>>>> actlrvar->actlrcfg); >>>>> + break; >>>>> + } >>>>> + } >>>>> + } >>>>> + >>>> >>>> This block and the one in qcom_adreno_smmu_init_context() are exactly >>>> the same. Possible to do some refactoring? >>>> >>> >>> I will check if this repeated blocks can be accomodated this into >>> qcom_smmu_set_actlr function if that would be fine. >>> >> >> Also adding to this, this might increase the number of indentation >> inside qcom_smmu_set_actlr as well, to around 5. So wouldn't this >> be an issue? > > By the way, we can refactor this: > > if (qsmmu->data->actlrvar) { > actlrvar = qsmmu->data->actlrvar; > for (; actlrvar->io_start; actlrvar++) { > if (actlrvar->io_start == smmu->ioaddr) { > qcom_smmu_set_actlr(dev, smmu, cbndx, actlrvar->actlrcfg); > break; > } > } > } > > into > > // add const u8 num_actlrcfgs to struct actrl_variant to > // save on sentinel space: > // sizeof(u8) < sizeof(ptr) + sizeof(resource_size_t) >
Git it, Would it be better to add this in struct qcom_smmu_match_data ? Posted a sample below.
> > [declarations] > const struct actlr_variant *actlrvar = qsmmu->data->actlrvar; > int i; > > [rest of the functions] > > if (!actlrvar) > return 0; > > for (i = 0; i < actrlvar->num_actrlcfgs; i++) { > if (actlrvar[i].io_start == smmu->ioaddr) { > qcom_smmu_set_actlr(dev, smmu, cbndx, actlrvar->actlrcfg); > break; > } > } > > Saving both on .TEXT size and indentation levels :) > Thanks for this suggestion Konrad, will try to implement this, as it would reduce the indent levels to good extent. Would something like this be okay?
static int qcom_smmu_init_context(struct arm_smmu_domain *smmu_domain, struct qcom_smmu *qsmmu = to_qcom_smmu(smmu); const struct actlr_variant *actlrvar; int cbndx = smmu_domain->cfg.cbndx; + int i;
+ actlrvar = qsmmu->data->actlrvar; + + if (!actlrvar) + goto end; + + for (i = 0; i < qsmmu->data->num_smmu ; i++) { + if (actlrvar[i].io_start == smmu->ioaddr) { + qcom_smmu_set_actlr(dev, smmu, cbndx, + actlrvar[i].actlrcfg); + break; } }
+end: smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
return 0;
Thanks & regards, Bibek
> Konrad
| |