drivers/gpu/drm/omapdrm/omap_gem.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
While a scatter-gather table having only 1 entry does imply it is
contiguous, it is a logic error to assume the inverse. Tables can have
more than 1 entry and still be contiguous. Use a proper check here.
Signed-off-by: Andrew Davis <afd@ti.com>
---
drivers/gpu/drm/omapdrm/omap_gem.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index cf571796fd26..52c00b795459 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -48,7 +48,7 @@ struct omap_gem_object {
* OMAP_BO_MEM_DMA_API flag set)
*
* - buffers imported from dmabuf (with the OMAP_BO_MEM_DMABUF flag set)
- * if they are physically contiguous (when sgt->orig_nents == 1)
+ * if they are physically contiguous
*
* - buffers mapped through the TILER when pin_cnt is not zero, in which
* case the DMA address points to the TILER aperture
@@ -148,12 +148,18 @@ u64 omap_gem_mmap_offset(struct drm_gem_object *obj)
return drm_vma_node_offset_addr(&obj->vma_node);
}
+static bool omap_gem_sgt_is_contiguous(struct sg_table *sgt, size)
+{
+ return !(drm_prime_get_contiguous_size(sgt) < size);
+}
+
static bool omap_gem_is_contiguous(struct omap_gem_object *omap_obj)
{
if (omap_obj->flags & OMAP_BO_MEM_DMA_API)
return true;
- if ((omap_obj->flags & OMAP_BO_MEM_DMABUF) && omap_obj->sgt->nents == 1)
+ if ((omap_obj->flags & OMAP_BO_MEM_DMABUF) &&
+ omap_gem_sgt_is_contiguous(omap_obj->sgt, omap_obj->base->size))
return true;
return false;
@@ -1398,7 +1404,7 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size,
union omap_gem_size gsize;
/* Without a DMM only physically contiguous buffers can be supported. */
- if (sgt->orig_nents != 1 && !priv->has_dmm)
+ if (!omap_gem_sgt_is_contiguous(sgt, size) && !priv->has_dmm)
return ERR_PTR(-EINVAL);
gsize.bytes = PAGE_ALIGN(size);
@@ -1412,7 +1418,7 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size,
omap_obj->sgt = sgt;
- if (sgt->orig_nents == 1) {
+ if (omap_gem_sgt_is_contiguous(sgt, size)) {
omap_obj->dma_addr = sg_dma_address(sgt->sgl);
} else {
/* Create pages list from sgt */
--
2.36.1On 8/22/22 7:03 PM, Andrew Davis wrote:
> While a scatter-gather table having only 1 entry does imply it is
> contiguous, it is a logic error to assume the inverse. Tables can have
> more than 1 entry and still be contiguous. Use a proper check here.
>
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
Looks like an old version of this patch got in my to-send queue,
please ignore this one, will send updated v2.
Thanks,
Andrew
> drivers/gpu/drm/omapdrm/omap_gem.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
> index cf571796fd26..52c00b795459 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
> @@ -48,7 +48,7 @@ struct omap_gem_object {
> * OMAP_BO_MEM_DMA_API flag set)
> *
> * - buffers imported from dmabuf (with the OMAP_BO_MEM_DMABUF flag set)
> - * if they are physically contiguous (when sgt->orig_nents == 1)
> + * if they are physically contiguous
> *
> * - buffers mapped through the TILER when pin_cnt is not zero, in which
> * case the DMA address points to the TILER aperture
> @@ -148,12 +148,18 @@ u64 omap_gem_mmap_offset(struct drm_gem_object *obj)
> return drm_vma_node_offset_addr(&obj->vma_node);
> }
>
> +static bool omap_gem_sgt_is_contiguous(struct sg_table *sgt, size)
> +{
> + return !(drm_prime_get_contiguous_size(sgt) < size);
> +}
> +
> static bool omap_gem_is_contiguous(struct omap_gem_object *omap_obj)
> {
> if (omap_obj->flags & OMAP_BO_MEM_DMA_API)
> return true;
>
> - if ((omap_obj->flags & OMAP_BO_MEM_DMABUF) && omap_obj->sgt->nents == 1)
> + if ((omap_obj->flags & OMAP_BO_MEM_DMABUF) &&
> + omap_gem_sgt_is_contiguous(omap_obj->sgt, omap_obj->base->size))
> return true;
>
> return false;
> @@ -1398,7 +1404,7 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size,
> union omap_gem_size gsize;
>
> /* Without a DMM only physically contiguous buffers can be supported. */
> - if (sgt->orig_nents != 1 && !priv->has_dmm)
> + if (!omap_gem_sgt_is_contiguous(sgt, size) && !priv->has_dmm)
> return ERR_PTR(-EINVAL);
>
> gsize.bytes = PAGE_ALIGN(size);
> @@ -1412,7 +1418,7 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size,
>
> omap_obj->sgt = sgt;
>
> - if (sgt->orig_nents == 1) {
> + if (omap_gem_sgt_is_contiguous(sgt, size)) {
> omap_obj->dma_addr = sg_dma_address(sgt->sgl);
> } else {
> /* Create pages list from sgt */
Hi Andrew,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on drm-intel/for-linux-next drm-tip/drm-tip linus/master v6.0-rc2 next-20220824]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Andrew-Davis/drm-omapdrm-Improve-check-for-contiguous-buffers/20220823-080542
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
config: arm-allyesconfig (https://download.01.org/0day-ci/archive/20220825/202208250645.sVXFPe8D-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/5eeaaada78b2038b8f2bf483926b4aee998016ff
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Andrew-Davis/drm-omapdrm-Improve-check-for-contiguous-buffers/20220823-080542
git checkout 5eeaaada78b2038b8f2bf483926b4aee998016ff
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/omapdrm/omap_gem.c:151:62: error: unknown type name 'size'; did you mean 'size_t'?
151 | static bool omap_gem_sgt_is_contiguous(struct sg_table *sgt, size)
| ^~~~
| size_t
drivers/gpu/drm/omapdrm/omap_gem.c: In function 'omap_gem_is_contiguous':
>> drivers/gpu/drm/omapdrm/omap_gem.c:162:13: error: implicit declaration of function 'omap_gem_sgt_is_contiguous'; did you mean 'omap_gem_is_contiguous'? [-Werror=implicit-function-declaration]
162 | omap_gem_sgt_is_contiguous(omap_obj->sgt, omap_obj->base->size))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
| omap_gem_is_contiguous
>> drivers/gpu/drm/omapdrm/omap_gem.c:162:69: error: invalid type argument of '->' (have 'struct drm_gem_object')
162 | omap_gem_sgt_is_contiguous(omap_obj->sgt, omap_obj->base->size))
| ^~
cc1: some warnings being treated as errors
vim +151 drivers/gpu/drm/omapdrm/omap_gem.c
150
> 151 static bool omap_gem_sgt_is_contiguous(struct sg_table *sgt, size)
152 {
153 return !(drm_prime_get_contiguous_size(sgt) < size);
154 }
155
156 static bool omap_gem_is_contiguous(struct omap_gem_object *omap_obj)
157 {
158 if (omap_obj->flags & OMAP_BO_MEM_DMA_API)
159 return true;
160
161 if ((omap_obj->flags & OMAP_BO_MEM_DMABUF) &&
> 162 omap_gem_sgt_is_contiguous(omap_obj->sgt, omap_obj->base->size))
163 return true;
164
165 return false;
166 }
167
--
0-DAY CI Kernel Test Service
https://01.org/lkp
© 2016 - 2026 Red Hat, Inc.