On 28.11.2023 04:47, Paz Zcharya wrote:
On Mon, Nov 27, 2023 at 8:20 PM Paz Zcharya <pazz@xxxxxxxxxxxx> wrote:
On 21.11.2023 13:06, Andrzej Hajda wrote:
On 18.11.2023 00:01, Paz Zcharya wrote:
On Tue, Nov 14, 2023 at 10:13:59PM -0500, Rodrigo Vivi wrote:
On Sun, Nov 05, 2023 at 05:27:03PM +0000, Paz Zcharya wrote:
Hi Rodrigo, thanks for the great comments.
Apologies for using a wrong/confusing terminology. I think 'phys_base'
is supposed to be the offset in the GEM BO, where base (or
"Surface Base Address") is supposed to be the GTT offset.
Since base is taken from PLANE_SURF register it should be resolvable via
GGTT to physical address pointing to actual framebuffer.
I couldn't find anything in the specs.
It was quite cryptic. I meant I have not found anything about assumption
from commit history that for iGPU there should be 1:1 mapping, this is why
there was an assignment "phys_base = base". Possibly the assumption is not
valid anymore for MTL(?).
Without the assumption we need to check GGTT to determine phys address.
The simplest approach would be then do the same as in case of DGFX:
gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm;
gen8_pte_t pte;
gte += base / I915_GTT_PAGE_SIZE;
pte = ioread64(gte);
phys_base = pte & I915_GTT_PAGE_MASK;
Regards
Andrzej
Hey Andrzej,
On a second thought, what do you think about something like
+ gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm;
+ gen8_pte_t pte;
+ gte += base / I915_GTT_PAGE_SIZE;
+ pte = ioread64(gte);
+ pte = pte & I915_GTT_PAGE_MASK;
+ phys_base = pte - i915->mm.stolen_region->region.start;
The only difference is the last line.
Bingo :) It seems to be generic algorithm to get phys_base for all
platforms:
- on older platforms stolen_region points to system memory which starts
at 0,
- on DG2 it uses lmem region which starts at 0 as well,
- on MTL stolen_region points to stolen-local which starts at 0x800000.
So this whole "if (IS_DGFX(i915)) {...} else {...}" could be replaced
with sth generic.
1. Find pte.
2. if(IS_DGFX(i915) && pte & GEN12_GGTT_PTE_LM) mem =
i915->mm.regions[INTEL_REGION_LMEM_0] else mem = i915->mm.stolen_region
3. phys_base = (pte & I915_GTT_PAGE_MASK) - mem->region.start;
Regards
Andrzej
Based on what I wrote before, I think `phys_base` is named incorrectly and
that it does not reflect the physical address, but the start offset of
i915->mm.stolen_region. So if we offset the start value of the stolen
region, this code looks correct to me (and it also works on my
MeteorLake device).
What do you think?
Many thanks,
Paz
