On Fri, Oct 29, 2021 at 05:26:05PM +0100, Matthew Auld wrote:
> On Tue, 26 Oct 2021 at 23:51, Imre Deak <imre.deak@xxxxxxxxx> wrote:
> >
> > Factor out functions needed to map contiguous FB obj pages to a GTT/DPT
> > VMA view in the next patch.
> >
> > No functional changes.
> >
> > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/gt/intel_ggtt.c | 110 +++++++++++++++------------
> > 1 file changed, 60 insertions(+), 50 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > index 57c97554393b9..a30366d4965b6 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > @@ -1387,6 +1387,25 @@ intel_rotate_pages(struct intel_rotation_info *rot_info,
> > return ERR_PTR(ret);
> > }
> >
> > +static struct scatterlist *
> > +add_padding_pages(unsigned int count,
> > + struct sg_table *st, struct scatterlist *sg)
> > +{
> > + st->nents++;
> > +
> > + /*
> > + * The DE ignores the PTEs for the padding tiles, the sg entry
> > + * here is just a convenience to indicate how many padding PTEs
> > + * to insert at this spot.
> > + */
> > + sg_set_page(sg, NULL, count * 4096, 0);
>
> s/4096/I915_GTT_PAGE_SIZE ?
Yes, will change this.
> > + sg_dma_address(sg) = 0;
>
> I guess maybe a little bit scary, since that might be a valid address.
Was pondering what to use here, but this looked the simple and fast way.
Using the scratch page (for all padding PTEs this sg covers) would
require special-casing this sg entry that the insert_entries hook could
see and repeat the same address for each PTE instead of the usual
increment-by-page-size for each sg iteration step. Or we could add here
'count' number of sg entries each pointing to the scratch page, but
that's not the most compact.
Another concern was the prefetching done by VTd, which may require a
valid PTE. But these PTEs go to DPT (vs. GGTT) and at least there the HW
is not using the padding PTEs for anything. (Bspec 53393): "The padded
entries do not need to be valid."
> Using the vma->vm scratch might be annoying though, since it could be
> a different type than the object, plus this is only the GGTT.
>
> Looks fine I think,
> Reviewed-by: Matthew Auld <matthew.auld@xxxxxxxxx>
Thanks.
