All of lore.kernel.org
 help / color / mirror / Atom feed
From: robdclark@gmail.com (Rob Clark)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/5] DRM: Armada: Add Armada DRM driver
Date: Thu, 10 Oct 2013 20:10:58 -0400	[thread overview]
Message-ID: <CAF6AEGvb-oeQqYF-XyZ1PjCaqojJ=DurLVGkZxUFwC_WveoTwg@mail.gmail.com> (raw)
In-Reply-To: <20131010225331.GT25034@n2100.arm.linux.org.uk>

On Thu, Oct 10, 2013 at 6:53 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Oct 10, 2013 at 06:23:26PM -0400, Rob Clark wrote:
>> On Thu, Oct 10, 2013 at 5:59 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Thu, Oct 10, 2013 at 05:25:15PM -0400, Rob Clark wrote:
>> >> probably this thread is applicable here too?
>> >>
>> >> https://lkml.org/lkml/2013/9/12/417
>> >>
>> >> (although.. we have at least a few  slightly differet variants on the
>> >> same errno -> VM_FAULT_x switch in different drivers.. maybe we should
>> >> do something better)
>> >
>> > Hmm.  It seems today's vm_insert_pfn() has the following error return
>> > values:
>> >
>> > -EFAULT - virtual address outside vma
>> > -EINVAL - track_pfn_insert() failure
>> > -ENOMEM - failed to get locked pte
>> > -EBUSY - entry already exists in pte
>> >
>> > So it seems my handling -EIO, -EAGAIN, -ERESTARTSYS and -EINTR are all
>> > redundant and can be removed.  I'm not sure if it makes sense for this
>> > to be generic - it looks like it's only Intel, gma500 and me who use
>> > this, and Intel handles more error codes (due to it calling other
>> > functions.)
>>
>> I just noticed msm and omapdrm are missing the -EBUSY case (have some
>> patches I need to send), which was why I mentioned this.  They do have
>> other error cases from other fxns, so maybe something generic/common
>> doesn't make sense..  but I realized i915 fixed the same issue a while
>> back, so somewhere common has the advantage of somehow not forgetting
>> to fix other drivers ;-)
>
> Here's the promised delta:

looks good.. I'll try to get to reviewing the rest of the series
tomorrow, or at least before the end of the weekend

Reviewed-by: Rob Clark <robdclark@gmail.com>

BR,
-R

>  drivers/gpu/drm/armada/armada_gem.c |    5 -----
>  1 files changed, 0 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c
> index 02de7a4..1e74f70 100644
> --- a/drivers/gpu/drm/armada/armada_gem.c
> +++ b/drivers/gpu/drm/armada/armada_gem.c
> @@ -25,12 +25,7 @@ static int armada_gem_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>         ret = vm_insert_pfn(vma, addr, pfn);
>
>         switch (ret) {
> -       case -EIO:
> -       case -EAGAIN:
> -               set_need_resched();
>         case 0:
> -       case -ERESTARTSYS:
> -       case -EINTR:
>         case -EBUSY:
>                 return VM_FAULT_NOPAGE;
>         case -ENOMEM:
>

WARNING: multiple messages have this Message-ID (diff)
From: Rob Clark <robdclark@gmail.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Jason Cooper <jason@lakedaemon.net>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Subject: Re: [PATCH 2/5] DRM: Armada: Add Armada DRM driver
Date: Thu, 10 Oct 2013 20:10:58 -0400	[thread overview]
Message-ID: <CAF6AEGvb-oeQqYF-XyZ1PjCaqojJ=DurLVGkZxUFwC_WveoTwg@mail.gmail.com> (raw)
In-Reply-To: <20131010225331.GT25034@n2100.arm.linux.org.uk>

On Thu, Oct 10, 2013 at 6:53 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Oct 10, 2013 at 06:23:26PM -0400, Rob Clark wrote:
>> On Thu, Oct 10, 2013 at 5:59 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Thu, Oct 10, 2013 at 05:25:15PM -0400, Rob Clark wrote:
>> >> probably this thread is applicable here too?
>> >>
>> >> https://lkml.org/lkml/2013/9/12/417
>> >>
>> >> (although.. we have at least a few  slightly differet variants on the
>> >> same errno -> VM_FAULT_x switch in different drivers.. maybe we should
>> >> do something better)
>> >
>> > Hmm.  It seems today's vm_insert_pfn() has the following error return
>> > values:
>> >
>> > -EFAULT - virtual address outside vma
>> > -EINVAL - track_pfn_insert() failure
>> > -ENOMEM - failed to get locked pte
>> > -EBUSY - entry already exists in pte
>> >
>> > So it seems my handling -EIO, -EAGAIN, -ERESTARTSYS and -EINTR are all
>> > redundant and can be removed.  I'm not sure if it makes sense for this
>> > to be generic - it looks like it's only Intel, gma500 and me who use
>> > this, and Intel handles more error codes (due to it calling other
>> > functions.)
>>
>> I just noticed msm and omapdrm are missing the -EBUSY case (have some
>> patches I need to send), which was why I mentioned this.  They do have
>> other error cases from other fxns, so maybe something generic/common
>> doesn't make sense..  but I realized i915 fixed the same issue a while
>> back, so somewhere common has the advantage of somehow not forgetting
>> to fix other drivers ;-)
>
> Here's the promised delta:

looks good.. I'll try to get to reviewing the rest of the series
tomorrow, or at least before the end of the weekend

Reviewed-by: Rob Clark <robdclark@gmail.com>

BR,
-R

>  drivers/gpu/drm/armada/armada_gem.c |    5 -----
>  1 files changed, 0 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c
> index 02de7a4..1e74f70 100644
> --- a/drivers/gpu/drm/armada/armada_gem.c
> +++ b/drivers/gpu/drm/armada/armada_gem.c
> @@ -25,12 +25,7 @@ static int armada_gem_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>         ret = vm_insert_pfn(vma, addr, pfn);
>
>         switch (ret) {
> -       case -EIO:
> -       case -EAGAIN:
> -               set_need_resched();
>         case 0:
> -       case -ERESTARTSYS:
> -       case -EINTR:
>         case -EBUSY:
>                 return VM_FAULT_NOPAGE;
>         case -ENOMEM:
>

  reply	other threads:[~2013-10-11  0:10 UTC|newest]

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-06 22:07 [PATCH 0/5] Armada DRM stuff Russell King - ARM Linux
2013-10-06 22:07 ` Russell King - ARM Linux
2013-10-06 22:07 ` [PATCH 1/5] drm/i2c: tda998x: set VIF for full range, underscanned display Russell King
2013-10-06 22:07   ` Russell King
2013-10-07  8:59   ` Jean-Francois Moine
2013-10-07  8:59     ` Jean-Francois Moine
2013-10-18 15:00     ` Rob Clark
2013-10-18 15:00       ` Rob Clark
2013-10-06 22:08 ` [PATCH 2/5] DRM: Armada: Add Armada DRM driver Russell King
2013-10-06 22:08   ` Russell King
2013-10-10 21:25   ` Rob Clark
2013-10-10 21:25     ` Rob Clark
2013-10-10 21:59     ` Russell King - ARM Linux
2013-10-10 21:59       ` Russell King - ARM Linux
2013-10-10 22:23       ` Rob Clark
2013-10-10 22:23         ` Rob Clark
2013-10-10 22:53         ` Russell King - ARM Linux
2013-10-10 22:53           ` Russell King - ARM Linux
2013-10-11  0:10           ` Rob Clark [this message]
2013-10-11  0:10             ` Rob Clark
2013-10-06 22:09 ` [PATCH 3/5] DRM: Armada: Add support for ARGB 32x64 or 64x32 hardware cursors Russell King
2013-10-06 22:09   ` Russell King
2013-10-07  9:01   ` Jean-Francois Moine
2013-10-07  9:01     ` Jean-Francois Moine
2013-10-07  9:40     ` Russell King - ARM Linux
2013-10-07  9:40       ` Russell King - ARM Linux
2013-10-07 10:09       ` Jean-Francois Moine
2013-10-07 10:09         ` Jean-Francois Moine
2013-10-07 10:32         ` Russell King - ARM Linux
2013-10-07 10:32           ` Russell King - ARM Linux
2013-10-07 12:29           ` Siarhei Siamashka
2013-10-07 12:29             ` Siarhei Siamashka
2013-10-07 12:50             ` Russell King - ARM Linux
2013-10-07 12:50               ` Russell King - ARM Linux
2013-10-17 23:58               ` Rob Clark
2013-10-17 23:58                 ` Rob Clark
2013-10-18 14:31                 ` Alex Deucher
2013-10-18 14:31                   ` Alex Deucher
2013-10-06 22:10 ` [PATCH 4/5] DRM: Armada: start of MMP2/MMP3 support Russell King
2013-10-06 22:10   ` Russell King
2013-10-18  0:11   ` Rob Clark
2013-10-18  0:11     ` Rob Clark
2013-10-06 22:11 ` [PATCH 5/5] DRM: Armada: add support for drm tda19988 driver Russell King
2013-10-06 22:11   ` Russell King
2013-10-07  9:18   ` Jean-Francois Moine
2013-10-07  9:18     ` Jean-Francois Moine
2013-10-07  9:44     ` Russell King - ARM Linux
2013-10-07  9:44       ` Russell King - ARM Linux
2013-10-07 10:48       ` Jean-Francois Moine
2013-10-07 10:48         ` Jean-Francois Moine
2013-10-07 11:09         ` Russell King - ARM Linux
2013-10-07 11:09           ` Russell King - ARM Linux
2013-10-07 11:29           ` Sebastian Hesselbarth
2013-10-07 11:29             ` Sebastian Hesselbarth
2013-10-07 15:53             ` Mark Brown
2013-10-07 15:53               ` Mark Brown
2013-10-07 16:08               ` Sebastian Hesselbarth
2013-10-07 16:08                 ` Sebastian Hesselbarth
2013-10-07 17:05                 ` Mark Brown
2013-10-07 17:05                   ` Mark Brown
2013-10-07 12:03           ` Jean-Francois Moine
2013-10-07 12:03             ` Jean-Francois Moine
2013-10-07 12:36             ` Russell King - ARM Linux
2013-10-07 12:36               ` Russell King - ARM Linux
2013-10-07 14:59           ` Rob Clark
2013-10-07 14:59             ` Rob Clark
2013-10-08  9:19             ` Jean-Francois Moine
2013-10-08  9:19               ` Jean-Francois Moine
2013-10-08  9:49               ` Russell King - ARM Linux
2013-10-08  9:49                 ` Russell King - ARM Linux
2013-10-08 15:34                 ` Jean-Francois Moine
2013-10-08 15:34                   ` Jean-Francois Moine
2013-10-18  0:20                   ` Rob Clark
2013-10-18  0:20                     ` Rob Clark
2013-10-08 12:07               ` Rob Clark
2013-10-08 12:07                 ` Rob Clark
2013-10-07 21:47 ` [PATCH 0/5] Armada DRM stuff Sebastian Hesselbarth
2013-10-07 21:47   ` Sebastian Hesselbarth
2013-10-09 14:31   ` Russell King - ARM Linux
2013-10-09 14:31     ` Russell King - ARM Linux
2013-10-09 14:48     ` Rob Clark
2013-10-09 14:48       ` Rob Clark
2013-10-18 15:15 ` [GIT PULL] Armada DRM support Russell King - ARM Linux
2013-10-18 15:15   ` Russell King - ARM Linux
2013-10-22 13:36   ` Russell King - ARM Linux
2013-10-22 13:36     ` Russell King - ARM Linux

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAF6AEGvb-oeQqYF-XyZ1PjCaqojJ=DurLVGkZxUFwC_WveoTwg@mail.gmail.com' \
    --to=robdclark@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.