Messages in this thread | | | Subject | Re: [PATCH 4/4] iommu/arm-smmu-v3: Remove cmpxchg() in arm_smmu_cmdq_issue_cmdlist() | From | John Garry <> | Date | Wed, 24 Jun 2020 09:15:16 +0100 |
| |
>> >> I'd say that GENMASK_INPUT_CHECK() should be able to handle a l=0 and >> h=unsigned value, so I doubt this warn. >> >> Using GENMASK((int)cmdq->q.llq.max_n_shift, 0) resolves it, but it looks >> like GENMASK_INPUT_CHECK() could be improved. > > That said, I think this particular case might be even better off dodging > GENMASK() entirely, by doing something like this first. Untested... > > Robin. > > ----->8----- > Subject: [PATCH] iommu/arm-smmu-v3: Streamline queue calculations > > Beyond the initial queue setup based on the log2 values from ID > registers, the log2 queue size is only ever used in the form of > (1 << max_n_shift) to repeatedly recalculate the number of queue > elements. Simply storing it in that form leads to slightly more > efficient code, particularly in the low-level queue accessors > where it counts most: > > add/remove: 0/0 grow/shrink: 1/7 up/down: 4/-120 (-116) > Function old new delta > arm_smmu_init_one_queue 360 364 +4 > arm_smmu_priq_thread 512 508 -4 > arm_smmu_evtq_thread 300 292 -8 > __arm_smmu_cmdq_poll_set_valid_map.isra 296 288 -8 > queue_remove_raw 180 164 -16 > arm_smmu_gerror_handler 732 716 -16 > arm_smmu_device_probe 4312 4284 -28 > arm_smmu_cmdq_issue_cmdlist 1892 1852 -40 > Total: Before=20135, After=20019, chg -0.58% > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> > ---
[...]
> } > > - smmu->evtq.q.llq.max_n_shift = min_t(u32, EVTQ_MAX_SZ_SHIFT, > - FIELD_GET(IDR1_EVTQS, reg)); > - smmu->priq.q.llq.max_n_shift = min_t(u32, PRIQ_MAX_SZ_SHIFT, > - FIELD_GET(IDR1_PRIQS, reg)); > + max_n_shift = min_t(u32, EVTQ_MAX_SZ_SHIFT, FIELD_GET(IDR1_EVTQS, reg)); > + smmu->evtq.q.llq.max_n = 1 << max_n_shift;
So I require the bitmask of this for the prod, which would be (max_n << 1) - 1.
I don't feel too strongly either way, and the other big changes in this series need to be considered first...
Thanks, John
> + > + max_n_shift = min_t(u32, PRIQ_MAX_SZ_SHIFT, FIELD_GET(IDR1_PRIQS, reg)); > + smmu->priq.q.llq.max_n = 1 << max_n_shift; > > /* SID/SSID sizes */ > smmu->ssid_bits = FIELD_GET(IDR1_SSIDSIZE, reg); >
| |