diff mbox

drm/exynos: Add check for IOMMU while passing physically continous memory flag

Message ID 1375355972-25276-1-git-send-email-vikas.sajjan@linaro.org
State New
Headers show

Commit Message

Vikas C Sajjan Aug. 1, 2013, 11:19 a.m. UTC 
While trying to get boot-logo up on exynos5420 SMDK which has eDP panel
connected with resolution 2560x1600, following error occured even with
IOMMU enabled:
[0.880000] [drm:lowlevel_buffer_allocate] *ERROR* failed to allocate buffer.
[0.890000] [drm] Initialized exynos 1.0.0 20110530 on minor 0

This patch fixes the issue by adding a check for IOMMU.

Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
Signed-off-by: Arun Kumar <arun.kk@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Tomasz Figa Aug. 1, 2013, 11:20 p.m. UTC | #1 
Hi Vikas,

On Thursday 01 of August 2013 16:49:32 Vikas Sajjan wrote:
> While trying to get boot-logo up on exynos5420 SMDK which has eDP panel
> connected with resolution 2560x1600, following error occured even with
> IOMMU enabled:
> [0.880000] [drm:lowlevel_buffer_allocate] *ERROR* failed to allocate
> buffer. [0.890000] [drm] Initialized exynos 1.0.0 20110530 on minor 0
> 
> This patch fixes the issue by adding a check for IOMMU.
> 
> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
> Signed-off-by: Arun Kumar <arun.kk@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index 8e60bd6..2a86666
> 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> @@ -16,6 +16,7 @@
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_crtc_helper.h>
> +#include <drm/exynos_drm.h>
> 
>  #include "exynos_drm_drv.h"
>  #include "exynos_drm_fb.h"
> @@ -143,6 +144,7 @@ static int exynos_drm_fbdev_create(struct
> drm_fb_helper *helper, struct platform_device *pdev = dev->platformdev;
>  	unsigned long size;
>  	int ret;
> +	unsigned int flag;
> 
>  	DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d\n",
>  			sizes->surface_width, sizes->surface_height,
> @@ -166,7 +168,12 @@ static int exynos_drm_fbdev_create(struct
> drm_fb_helper *helper, size = mode_cmd.pitches[0] * mode_cmd.height;
> 
>  	/* 0 means to allocate physically continuous memory */
> -	exynos_gem_obj = exynos_drm_gem_create(dev, 0, size);
> +	if (!is_drm_iommu_supported(dev))
> +		flag = 0;
> +	else
> +		flag = EXYNOS_BO_NONCONTIG;

While noncontig memory might be used for devices that support IOMMU, there 
should be no problem with using contig memory for them, so this seems more 
like masking the original problem rather than tracking it down.

Could you check why the allocation fails when requesting contiguous 
memory?

Best regards,
Tomasz
Rob Clark Aug. 2, 2013, 12:33 a.m. UTC | #2 
On Thu, Aug 1, 2013 at 7:20 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi Vikas,
>
> On Thursday 01 of August 2013 16:49:32 Vikas Sajjan wrote:
>> While trying to get boot-logo up on exynos5420 SMDK which has eDP panel
>> connected with resolution 2560x1600, following error occured even with
>> IOMMU enabled:
>> [0.880000] [drm:lowlevel_buffer_allocate] *ERROR* failed to allocate
>> buffer. [0.890000] [drm] Initialized exynos 1.0.0 20110530 on minor 0
>>
>> This patch fixes the issue by adding a check for IOMMU.
>>
>> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
>> Signed-off-by: Arun Kumar <arun.kk@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c |    9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index 8e60bd6..2a86666
>> 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> @@ -16,6 +16,7 @@
>>  #include <drm/drm_crtc.h>
>>  #include <drm/drm_fb_helper.h>
>>  #include <drm/drm_crtc_helper.h>
>> +#include <drm/exynos_drm.h>
>>
>>  #include "exynos_drm_drv.h"
>>  #include "exynos_drm_fb.h"
>> @@ -143,6 +144,7 @@ static int exynos_drm_fbdev_create(struct
>> drm_fb_helper *helper, struct platform_device *pdev = dev->platformdev;
>>       unsigned long size;
>>       int ret;
>> +     unsigned int flag;
>>
>>       DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d\n",
>>                       sizes->surface_width, sizes->surface_height,
>> @@ -166,7 +168,12 @@ static int exynos_drm_fbdev_create(struct
>> drm_fb_helper *helper, size = mode_cmd.pitches[0] * mode_cmd.height;
>>
>>       /* 0 means to allocate physically continuous memory */
>> -     exynos_gem_obj = exynos_drm_gem_create(dev, 0, size);
>> +     if (!is_drm_iommu_supported(dev))
>> +             flag = 0;
>> +     else
>> +             flag = EXYNOS_BO_NONCONTIG;
>
> While noncontig memory might be used for devices that support IOMMU, there
> should be no problem with using contig memory for them, so this seems more
> like masking the original problem rather than tracking it down.

it is probably a good idea to not require contig memory when it is not
needed for performance or functionality (and if it is only
performance, then fallback gracefully to non-contig).. but yeah, would
be good to know if this is masking another issue all the same

BR,
-R

> Could you check why the allocation fails when requesting contiguous
> memory?
>
> Best regards,
> Tomasz
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vikas C Sajjan Aug. 2, 2013, 3:53 a.m. UTC | #3 
Hi Tomasz,


On 2 August 2013 04:50, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>
> Hi Vikas,
>
> On Thursday 01 of August 2013 16:49:32 Vikas Sajjan wrote:
> > While trying to get boot-logo up on exynos5420 SMDK which has eDP panel
> > connected with resolution 2560x1600, following error occured even with
> > IOMMU enabled:
> > [0.880000] [drm:lowlevel_buffer_allocate] *ERROR* failed to allocate
> > buffer. [0.890000] [drm] Initialized exynos 1.0.0 20110530 on minor 0
> >
> > This patch fixes the issue by adding a check for IOMMU.
> >
> > Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
> > Signed-off-by: Arun Kumar <arun.kk@samsung.com>
> > ---
> >  drivers/gpu/drm/exynos/exynos_drm_fbdev.c |    9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> > b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index 8e60bd6..2a86666
> > 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> > @@ -16,6 +16,7 @@
> >  #include <drm/drm_crtc.h>
> >  #include <drm/drm_fb_helper.h>
> >  #include <drm/drm_crtc_helper.h>
> > +#include <drm/exynos_drm.h>
> >
> >  #include "exynos_drm_drv.h"
> >  #include "exynos_drm_fb.h"
> > @@ -143,6 +144,7 @@ static int exynos_drm_fbdev_create(struct
> > drm_fb_helper *helper, struct platform_device *pdev = dev->platformdev;
> >       unsigned long size;
> >       int ret;
> > +     unsigned int flag;
> >
> >       DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d\n",
> >                       sizes->surface_width, sizes->surface_height,
> > @@ -166,7 +168,12 @@ static int exynos_drm_fbdev_create(struct
> > drm_fb_helper *helper, size = mode_cmd.pitches[0] * mode_cmd.height;
> >
> >       /* 0 means to allocate physically continuous memory */
> > -     exynos_gem_obj = exynos_drm_gem_create(dev, 0, size);
> > +     if (!is_drm_iommu_supported(dev))
> > +             flag = 0;
> > +     else
> > +             flag = EXYNOS_BO_NONCONTIG;
>
> While noncontig memory might be used for devices that support IOMMU, there
> should be no problem with using contig memory for them, so this seems more
> like masking the original problem rather than tracking it down.
>
> Could you check why the allocation fails when requesting contiguous
> memory?
>

It is failing if i am giving bigger resolution like 2560x1600, but if
i try for 1280x780 resolution, it   works fine.
Looks like arm_dma_alloc() is NOT able to allocate bigger buffer of
size 2560 * 1600 * 4.
Hence i used flag = EXYNOS_BO_NONCONTIG;


>
> Best regards,
> Tomasz
>
Sachin Kamat Aug. 2, 2013, 5:01 a.m. UTC | #4 
Hi Vikas,

On 2 August 2013 09:23, Vikas Sajjan <vikas.sajjan@linaro.org> wrote:
> Hi Tomasz,
>
>
> On 2 August 2013 04:50, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>>
>> Hi Vikas,
>>
>> On Thursday 01 of August 2013 16:49:32 Vikas Sajjan wrote:
>> > While trying to get boot-logo up on exynos5420 SMDK which has eDP panel
>> > connected with resolution 2560x1600, following error occured even with
>> > IOMMU enabled:
>> > [0.880000] [drm:lowlevel_buffer_allocate] *ERROR* failed to allocate
>> > buffer. [0.890000] [drm] Initialized exynos 1.0.0 20110530 on minor 0
>> >
>> > This patch fixes the issue by adding a check for IOMMU.
>> >
>> > Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
>> > Signed-off-by: Arun Kumar <arun.kk@samsung.com>
>> > ---
>> >  drivers/gpu/drm/exynos/exynos_drm_fbdev.c |    9 ++++++++-
>> >  1 file changed, 8 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> > b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index 8e60bd6..2a86666
>> > 100644
>> > --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> > +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> > @@ -16,6 +16,7 @@
>> >  #include <drm/drm_crtc.h>
>> >  #include <drm/drm_fb_helper.h>
>> >  #include <drm/drm_crtc_helper.h>
>> > +#include <drm/exynos_drm.h>
>> >
>> >  #include "exynos_drm_drv.h"
>> >  #include "exynos_drm_fb.h"
>> > @@ -143,6 +144,7 @@ static int exynos_drm_fbdev_create(struct
>> > drm_fb_helper *helper, struct platform_device *pdev = dev->platformdev;
>> >       unsigned long size;
>> >       int ret;
>> > +     unsigned int flag;
>> >
>> >       DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d\n",
>> >                       sizes->surface_width, sizes->surface_height,
>> > @@ -166,7 +168,12 @@ static int exynos_drm_fbdev_create(struct
>> > drm_fb_helper *helper, size = mode_cmd.pitches[0] * mode_cmd.height;
>> >
>> >       /* 0 means to allocate physically continuous memory */
>> > -     exynos_gem_obj = exynos_drm_gem_create(dev, 0, size);
>> > +     if (!is_drm_iommu_supported(dev))
>> > +             flag = 0;
>> > +     else
>> > +             flag = EXYNOS_BO_NONCONTIG;
>>
>> While noncontig memory might be used for devices that support IOMMU, there
>> should be no problem with using contig memory for them, so this seems more
>> like masking the original problem rather than tracking it down.
>>
>> Could you check why the allocation fails when requesting contiguous
>> memory?
>>
>
> It is failing if i am giving bigger resolution like 2560x1600, but if
> i try for 1280x780 resolution, it   works fine.
> Looks like arm_dma_alloc() is NOT able to allocate bigger buffer of
> size 2560 * 1600 * 4.

You may need to increase the zoneorder (from current default 11 to say
13) if you need large physically contiguous pages.
However I am not sure if it is recommended.
Sachin Kamat Aug. 2, 2013, 5:06 a.m. UTC | #5 
Hi Vikas,

On 1 August 2013 16:49, Vikas Sajjan <vikas.sajjan@linaro.org> wrote:
> While trying to get boot-logo up on exynos5420 SMDK which has eDP panel
> connected with resolution 2560x1600, following error occured even with
> IOMMU enabled:
> [0.880000] [drm:lowlevel_buffer_allocate] *ERROR* failed to allocate buffer.
> [0.890000] [drm] Initialized exynos 1.0.0 20110530 on minor 0
>
> This patch fixes the issue by adding a check for IOMMU.
>
> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
> Signed-off-by: Arun Kumar <arun.kk@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> index 8e60bd6..2a86666 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> @@ -16,6 +16,7 @@
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_crtc_helper.h>
> +#include <drm/exynos_drm.h>
>
>  #include "exynos_drm_drv.h"
>  #include "exynos_drm_fb.h"
> @@ -143,6 +144,7 @@ static int exynos_drm_fbdev_create(struct drm_fb_helper *helper,
>         struct platform_device *pdev = dev->platformdev;
>         unsigned long size;
>         int ret;
> +       unsigned int flag;
>
>         DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d\n",
>                         sizes->surface_width, sizes->surface_height,
> @@ -166,7 +168,12 @@ static int exynos_drm_fbdev_create(struct drm_fb_helper *helper,
>         size = mode_cmd.pitches[0] * mode_cmd.height;
>
>         /* 0 means to allocate physically continuous memory */

This comment is now wrongly placed. Please use EXYNOS_BO_CONTIG instead of 0
and get rid of this comment altogether.

> -       exynos_gem_obj = exynos_drm_gem_create(dev, 0, size);
> +       if (!is_drm_iommu_supported(dev))
> +               flag = 0;
> +       else
> +               flag = EXYNOS_BO_NONCONTIG;
> +
> +       exynos_gem_obj = exynos_drm_gem_create(dev, flag, size);
>         if (IS_ERR(exynos_gem_obj)) {
>                 ret = PTR_ERR(exynos_gem_obj);
>                 goto err_release_framebuffer;
Vikas C Sajjan Aug. 2, 2013, 6:38 a.m. UTC | #6 
Hi Rob,

On 2 August 2013 06:03, Rob Clark <robdclark@gmail.com> wrote:
> On Thu, Aug 1, 2013 at 7:20 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>> Hi Vikas,
>>
>> On Thursday 01 of August 2013 16:49:32 Vikas Sajjan wrote:
>>> While trying to get boot-logo up on exynos5420 SMDK which has eDP panel
>>> connected with resolution 2560x1600, following error occured even with
>>> IOMMU enabled:
>>> [0.880000] [drm:lowlevel_buffer_allocate] *ERROR* failed to allocate
>>> buffer. [0.890000] [drm] Initialized exynos 1.0.0 20110530 on minor 0
>>>
>>> This patch fixes the issue by adding a check for IOMMU.
>>>
>>> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
>>> Signed-off-by: Arun Kumar <arun.kk@samsung.com>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c |    9 ++++++++-
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>>> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index 8e60bd6..2a86666
>>> 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>>> @@ -16,6 +16,7 @@
>>>  #include <drm/drm_crtc.h>
>>>  #include <drm/drm_fb_helper.h>
>>>  #include <drm/drm_crtc_helper.h>
>>> +#include <drm/exynos_drm.h>
>>>
>>>  #include "exynos_drm_drv.h"
>>>  #include "exynos_drm_fb.h"
>>> @@ -143,6 +144,7 @@ static int exynos_drm_fbdev_create(struct
>>> drm_fb_helper *helper, struct platform_device *pdev = dev->platformdev;
>>>       unsigned long size;
>>>       int ret;
>>> +     unsigned int flag;
>>>
>>>       DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d\n",
>>>                       sizes->surface_width, sizes->surface_height,
>>> @@ -166,7 +168,12 @@ static int exynos_drm_fbdev_create(struct
>>> drm_fb_helper *helper, size = mode_cmd.pitches[0] * mode_cmd.height;
>>>
>>>       /* 0 means to allocate physically continuous memory */
>>> -     exynos_gem_obj = exynos_drm_gem_create(dev, 0, size);
>>> +     if (!is_drm_iommu_supported(dev))
>>> +             flag = 0;
>>> +     else
>>> +             flag = EXYNOS_BO_NONCONTIG;
>>
>> While noncontig memory might be used for devices that support IOMMU, there
>> should be no problem with using contig memory for them, so this seems more
>> like masking the original problem rather than tracking it down.
>
> it is probably a good idea to not require contig memory when it is not
> needed for performance or functionality (and if it is only
> performance, then fallback gracefully to non-contig).. but yeah, would
> be good to know if this is masking another issue all the same
>

Whats happening with CONTIG flag and with IOMMU,  is

 __iommu_alloc_buffer() ---> dma_alloc_from_contiguous() and in this
function it fails at
this condition check  if (pageno >= cma->count)

So I tried increasing the CONFIG_CMA_SIZE_MBYTES to 24,  this check
succeeds and it works well without my patch.

But what about the case where CONFIG_CMA is disabled , yet i want
bigger memory for a device.
 I think using IOMMU we can achieve this.

 correct me, if i am wrong.


> BR,
> -R
>
>> Could you check why the allocation fails when requesting contiguous
>> memory?
>>
>> Best regards,
>> Tomasz
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-media" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomasz Figa Aug. 2, 2013, 7:23 a.m. UTC | #7 
Hi Vikas,

On Friday 02 of August 2013 12:08:52 Vikas Sajjan wrote:
> Hi Rob,
> 
> On 2 August 2013 06:03, Rob Clark <robdclark@gmail.com> wrote:
> > On Thu, Aug 1, 2013 at 7:20 PM, Tomasz Figa <tomasz.figa@gmail.com> 
wrote:
> >> Hi Vikas,
> >> 
> >> On Thursday 01 of August 2013 16:49:32 Vikas Sajjan wrote:
> >>> While trying to get boot-logo up on exynos5420 SMDK which has eDP
> >>> panel
> >>> connected with resolution 2560x1600, following error occured even
> >>> with
> >>> IOMMU enabled:
> >>> [0.880000] [drm:lowlevel_buffer_allocate] *ERROR* failed to allocate
> >>> buffer. [0.890000] [drm] Initialized exynos 1.0.0 20110530 on minor
> >>> 0
> >>> 
> >>> This patch fixes the issue by adding a check for IOMMU.
> >>> 
> >>> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
> >>> Signed-off-by: Arun Kumar <arun.kk@samsung.com>
> >>> ---
> >>> 
> >>>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c |    9 ++++++++-
> >>>  1 file changed, 8 insertions(+), 1 deletion(-)
[snip]
> >>> @@ -166,7 +168,12 @@ static int exynos_drm_fbdev_create(struct
> >>> drm_fb_helper *helper, size = mode_cmd.pitches[0] * mode_cmd.height;
> >>> 
> >>>       /* 0 means to allocate physically continuous memory */
> >>> 
> >>> -     exynos_gem_obj = exynos_drm_gem_create(dev, 0, size);
> >>> +     if (!is_drm_iommu_supported(dev))
> >>> +             flag = 0;
> >>> +     else
> >>> +             flag = EXYNOS_BO_NONCONTIG;
> >> 
> >> While noncontig memory might be used for devices that support IOMMU,
> >> there should be no problem with using contig memory for them, so
> >> this seems more like masking the original problem rather than
> >> tracking it down.> 
> > it is probably a good idea to not require contig memory when it is not
> > needed for performance or functionality (and if it is only
> > performance, then fallback gracefully to non-contig).. but yeah, would
> > be good to know if this is masking another issue all the same
> 
> Whats happening with CONTIG flag and with IOMMU,  is
> 
>  __iommu_alloc_buffer() ---> dma_alloc_from_contiguous() and in this
> function it fails at
> this condition check  if (pageno >= cma->count)
> 
> So I tried increasing the CONFIG_CMA_SIZE_MBYTES to 24,  this check
> succeeds and it works well without my patch.
> 
> But what about the case where CONFIG_CMA is disabled , yet i want
> bigger memory for a device.
>  I think using IOMMU we can achieve this.
>
>  correct me, if i am wrong.

This is probably fine. I'm not sure about performance aspects of using 
noncontig memory as framebuffer, though. This needs to be checked and if 
there is some performance penalty, I would make noncontig allocation a 
fallback case, if contig fails, as Rob has suggested.

Also I think you should adjust the commit message to say that non-
contiguous memory can be used when IOMMU is supported, so there is no need 
to force contiguous allocations, since this is not a bug fix, but rather a 
feature this patch is adding.

Best regards,
Tomasz
 
> > BR,
> > -R
> > 
> >> Could you check why the allocation fails when requesting contiguous
> >> memory?
> >> 
> >> Best regards,
> >> Tomasz
> >> 
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe
> >> linux-media" in the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
대인기/Tizen Platform Lab(SR)/삼성전자 Aug. 2, 2013, 7:28 a.m. UTC | #8 
2013/8/2 Vikas Sajjan <vikas.sajjan@linaro.org>

> Hi Rob,
>
> On 2 August 2013 06:03, Rob Clark <robdclark@gmail.com> wrote:
> > On Thu, Aug 1, 2013 at 7:20 PM, Tomasz Figa <tomasz.figa@gmail.com>
> wrote:
> >> Hi Vikas,
> >>
> >> On Thursday 01 of August 2013 16:49:32 Vikas Sajjan wrote:
> >>> While trying to get boot-logo up on exynos5420 SMDK which has eDP panel
> >>> connected with resolution 2560x1600, following error occured even with
> >>> IOMMU enabled:
> >>> [0.880000] [drm:lowlevel_buffer_allocate] *ERROR* failed to allocate
> >>> buffer. [0.890000] [drm] Initialized exynos 1.0.0 20110530 on minor 0
> >>>
> >>> This patch fixes the issue by adding a check for IOMMU.
> >>>
> >>> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
> >>> Signed-off-by: Arun Kumar <arun.kk@samsung.com>
> >>> ---
> >>>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c |    9 ++++++++-
> >>>  1 file changed, 8 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> >>> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index 8e60bd6..2a86666
> >>> 100644
> >>> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> >>> @@ -16,6 +16,7 @@
> >>>  #include <drm/drm_crtc.h>
> >>>  #include <drm/drm_fb_helper.h>
> >>>  #include <drm/drm_crtc_helper.h>
> >>> +#include <drm/exynos_drm.h>
> >>>
> >>>  #include "exynos_drm_drv.h"
> >>>  #include "exynos_drm_fb.h"
> >>> @@ -143,6 +144,7 @@ static int exynos_drm_fbdev_create(struct
> >>> drm_fb_helper *helper, struct platform_device *pdev = dev->platformdev;
> >>>       unsigned long size;
> >>>       int ret;
> >>> +     unsigned int flag;
> >>>
> >>>       DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d\n",
> >>>                       sizes->surface_width, sizes->surface_height,
> >>> @@ -166,7 +168,12 @@ static int exynos_drm_fbdev_create(struct
> >>> drm_fb_helper *helper, size = mode_cmd.pitches[0] * mode_cmd.height;
> >>>
> >>>       /* 0 means to allocate physically continuous memory */
> >>> -     exynos_gem_obj = exynos_drm_gem_create(dev, 0, size);
> >>> +     if (!is_drm_iommu_supported(dev))
> >>> +             flag = 0;
> >>> +     else
> >>> +             flag = EXYNOS_BO_NONCONTIG;
> >>
> >> While noncontig memory might be used for devices that support IOMMU,
> there
> >> should be no problem with using contig memory for them, so this seems
> more
> >> like masking the original problem rather than tracking it down.
> >
> > it is probably a good idea to not require contig memory when it is not
> > needed for performance or functionality (and if it is only
> > performance, then fallback gracefully to non-contig).. but yeah, would
> > be good to know if this is masking another issue all the same
> >
>
> Whats happening with CONTIG flag and with IOMMU,  is
>
>  __iommu_alloc_buffer() ---> dma_alloc_from_contiguous() and in this
> function it fails at
> this condition check  if (pageno >= cma->count)
>
> So I tried increasing the CONFIG_CMA_SIZE_MBYTES to 24,  this check
> succeeds and it works well without my patch.
>
> But what about the case where CONFIG_CMA is disabled , yet i want
> bigger memory for a device.
>  I think using IOMMU we can achieve this.
>
>  correct me, if i am wrong.
>
>
I'm on summer vacation so I'm afraid that I cannot test and look into it
but I guess you guy didn't declare CMA region for Exynos drm. And in this
case, the size of CMA declared region is 16MB as default. That is why works
well after increasing default size, CONFIG_CMA_SIZE_MBYTES, to 24MB. And I
mentioned long time ago, we are required to use physically contiguous
memory in case that bootloader uses physically contiguous memory for its
own framebuffer, and kernel wants to share the bootloader's framebuffer
region to resolve flickering issue while booted; that is required for
product. And one question, is there any reason that you guy should use
non-contiguous memory for framebuffer with iommu?

Thanks,
Inki Dae


>
> > BR,
> > -R
> >
> >> Could you check why the allocation fails when requesting contiguous
> >> memory?
> >>
> >> Best regards,
> >> Tomasz
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-media"
> in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Thanks and Regards
>  Vikas Sajjan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 8e60bd6..2a86666 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -16,6 +16,7 @@ 
 #include <drm/drm_crtc.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/exynos_drm.h>
 
 #include "exynos_drm_drv.h"
 #include "exynos_drm_fb.h"
@@ -143,6 +144,7 @@  static int exynos_drm_fbdev_create(struct drm_fb_helper *helper,
 	struct platform_device *pdev = dev->platformdev;
 	unsigned long size;
 	int ret;
+	unsigned int flag;
 
 	DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d\n",
 			sizes->surface_width, sizes->surface_height,
@@ -166,7 +168,12 @@  static int exynos_drm_fbdev_create(struct drm_fb_helper *helper,
 	size = mode_cmd.pitches[0] * mode_cmd.height;
 
 	/* 0 means to allocate physically continuous memory */
-	exynos_gem_obj = exynos_drm_gem_create(dev, 0, size);
+	if (!is_drm_iommu_supported(dev))
+		flag = 0;
+	else
+		flag = EXYNOS_BO_NONCONTIG;
+
+	exynos_gem_obj = exynos_drm_gem_create(dev, flag, size);
 	if (IS_ERR(exynos_gem_obj)) {
 		ret = PTR_ERR(exynos_gem_obj);
 		goto err_release_framebuffer;