| Message ID | 1360423656-10816-2-git-send-email-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Imre! On Sat, Feb 09, 2013 at 05:27:33PM +0200, Imre Deak wrote: > Add a helper to walk through a scatter list a page at a time. Needed by > upcoming patches fixing the scatter list walking logic in the i915 driver. Nice patch, but I think this would make a rather nice addition to the common scatterlist api in scatterlist.h, maybe called sg_page_iter. There's already another helper which does cpu mappings, but it has a different use-case (gives you the page mapped, which we don't neeed and can cope with not page-aligned sg tables). With dma-buf using sg tables I expect more users of such a sg page iterator to pop up. Most possible users of this will hang around on linaro-mm-sig, so please also cc that besides the usual suspects. Cheers, Daniel > > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > include/drm/drmP.h | 44 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 44 insertions(+) > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index fad21c9..0c0c213 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -1578,6 +1578,50 @@ extern int drm_sg_alloc_ioctl(struct drm_device *dev, void *data, > extern int drm_sg_alloc(struct drm_device *dev, struct drm_scatter_gather * request); > extern int drm_sg_free(struct drm_device *dev, void *data, > struct drm_file *file_priv); > +struct drm_sg_iter { > + struct scatterlist *sg; > + int sg_offset; Imo using sg_pfn_offset (i.e. sg_offset >> PAGE_SIZE) would make it clearer that this is all about iterating page-aligned sg tables. > + struct page *page; > +}; > + > +static inline int > +__drm_sg_iter_seek(struct drm_sg_iter *iter) > +{ > + while (iter->sg && iter->sg_offset >= iter->sg->length) { > + iter->sg_offset -= iter->sg->length; > + iter->sg = sg_next(iter->sg); And adding a WARN_ON(sg->legnth & ~PAGE_MASK); here would enforce that. > + } > + > + return iter->sg ? 0 : -1; > +} > + > +static inline struct page * > +drm_sg_iter_next(struct drm_sg_iter *iter) > +{ > + struct page *page; > + > + if (__drm_sg_iter_seek(iter)) > + return NULL; > + > + page = nth_page(sg_page(iter->sg), iter->sg_offset >> PAGE_SHIFT); > + iter->sg_offset = (iter->sg_offset + PAGE_SIZE) & PAGE_MASK; > + > + return page; > +} > + > +static inline struct page * > +drm_sg_iter_start(struct drm_sg_iter *iter, struct scatterlist *sg, > + unsigned long offset) > +{ > + iter->sg = sg; > + iter->sg_offset = offset; > + > + return drm_sg_iter_next(iter); > +} > + > +#define drm_for_each_sg_page(iter, sg, pgoffset) \ > + for ((iter)->page = drm_sg_iter_start((iter), (sg), (pgoffset));\ > + (iter)->page; (iter)->page = drm_sg_iter_next(iter)) Again, for the initialization I'd go with page numbers, not an offset in bytes. > > /* ATI PCIGART support (ati_pcigart.h) */ > extern int drm_ati_pcigart_init(struct drm_device *dev, > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Sat, 2013-02-09 at 19:56 +0100, Daniel Vetter wrote: > Hi Imre! > > On Sat, Feb 09, 2013 at 05:27:33PM +0200, Imre Deak wrote: > > Add a helper to walk through a scatter list a page at a time. Needed by > > upcoming patches fixing the scatter list walking logic in the i915 driver. > > Nice patch, but I think this would make a rather nice addition to the > common scatterlist api in scatterlist.h, maybe called sg_page_iter. > There's already another helper which does cpu mappings, but it has a > different use-case (gives you the page mapped, which we don't neeed and > can cope with not page-aligned sg tables). With dma-buf using sg tables I > expect more users of such a sg page iterator to pop up. Most possible > users of this will hang around on linaro-mm-sig, so please also cc that > besides the usual suspects. Ok, I wanted to sneak this in to drm (and win some time) thinking there isn't any other user of it atm. But I agree the ideal place for it is in scatterlist.h, so will send it there. > Cheers, Daniel > > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > include/drm/drmP.h | 44 ++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 44 insertions(+) > > > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > > index fad21c9..0c0c213 100644 > > --- a/include/drm/drmP.h > > +++ b/include/drm/drmP.h > > @@ -1578,6 +1578,50 @@ extern int drm_sg_alloc_ioctl(struct drm_device *dev, void *data, > > extern int drm_sg_alloc(struct drm_device *dev, struct drm_scatter_gather * request); > > extern int drm_sg_free(struct drm_device *dev, void *data, > > struct drm_file *file_priv); > > +struct drm_sg_iter { > > + struct scatterlist *sg; > > + int sg_offset; > > Imo using sg_pfn_offset (i.e. sg_offset >> PAGE_SIZE) would make it > clearer that this is all about iterating page-aligned sg tables. > > > + struct page *page; > > +}; > > + > > +static inline int > > +__drm_sg_iter_seek(struct drm_sg_iter *iter) > > +{ > > + while (iter->sg && iter->sg_offset >= iter->sg->length) { > > + iter->sg_offset -= iter->sg->length; > > + iter->sg = sg_next(iter->sg); > > And adding a WARN_ON(sg->legnth & ~PAGE_MASK); here would enforce that. > > > + } > > + > > + return iter->sg ? 0 : -1; > > +} > > + > > +static inline struct page * > > +drm_sg_iter_next(struct drm_sg_iter *iter) > > +{ > > + struct page *page; > > + > > + if (__drm_sg_iter_seek(iter)) > > + return NULL; > > + > > + page = nth_page(sg_page(iter->sg), iter->sg_offset >> PAGE_SHIFT); > > + iter->sg_offset = (iter->sg_offset + PAGE_SIZE) & PAGE_MASK; > > + > > + return page; > > +} > > + > > +static inline struct page * > > +drm_sg_iter_start(struct drm_sg_iter *iter, struct scatterlist *sg, > > + unsigned long offset) > > +{ > > + iter->sg = sg; > > + iter->sg_offset = offset; > > + > > + return drm_sg_iter_next(iter); > > +} > > + > > +#define drm_for_each_sg_page(iter, sg, pgoffset) \ > > + for ((iter)->page = drm_sg_iter_start((iter), (sg), (pgoffset));\ > > + (iter)->page; (iter)->page = drm_sg_iter_next(iter)) > > Again, for the initialization I'd go with page numbers, not an offset in > bytes. Ok, can change it to use page numbers instead. --Imre
diff --git a/include/drm/drmP.h b/include/drm/drmP.h index fad21c9..0c0c213 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1578,6 +1578,50 @@ extern int drm_sg_alloc_ioctl(struct drm_device *dev, void *data, extern int drm_sg_alloc(struct drm_device *dev, struct drm_scatter_gather * request); extern int drm_sg_free(struct drm_device *dev, void *data, struct drm_file *file_priv); +struct drm_sg_iter { + struct scatterlist *sg; + int sg_offset; + struct page *page; +}; + +static inline int +__drm_sg_iter_seek(struct drm_sg_iter *iter) +{ + while (iter->sg && iter->sg_offset >= iter->sg->length) { + iter->sg_offset -= iter->sg->length; + iter->sg = sg_next(iter->sg); + } + + return iter->sg ? 0 : -1; +} + +static inline struct page * +drm_sg_iter_next(struct drm_sg_iter *iter) +{ + struct page *page; + + if (__drm_sg_iter_seek(iter)) + return NULL; + + page = nth_page(sg_page(iter->sg), iter->sg_offset >> PAGE_SHIFT); + iter->sg_offset = (iter->sg_offset + PAGE_SIZE) & PAGE_MASK; + + return page; +} + +static inline struct page * +drm_sg_iter_start(struct drm_sg_iter *iter, struct scatterlist *sg, + unsigned long offset) +{ + iter->sg = sg; + iter->sg_offset = offset; + + return drm_sg_iter_next(iter); +} + +#define drm_for_each_sg_page(iter, sg, pgoffset) \ + for ((iter)->page = drm_sg_iter_start((iter), (sg), (pgoffset));\ + (iter)->page; (iter)->page = drm_sg_iter_next(iter)) /* ATI PCIGART support (ati_pcigart.h) */ extern int drm_ati_pcigart_init(struct drm_device *dev,
Add a helper to walk through a scatter list a page at a time. Needed by upcoming patches fixing the scatter list walking logic in the i915 driver. Signed-off-by: Imre Deak <imre.deak@intel.com> --- include/drm/drmP.h | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+)