[PATCH] drm/i915/skl: Fix up positive error code

[PATCH] drm/i915/skl: Fix up positive error code

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

It should have been negative since it is returned with ERR_PTR().

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index fc56c11..9903bb0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2540,7 +2540,7 @@ intel_rotate_fb_obj_pages(struct i915_ggtt_view *ggtt_view,
 	struct sg_table *st;
 	unsigned int tile_pitch, tile_height;
 	unsigned int width_pages, height_pages;
-	int ret = ENOMEM;
+	int ret = -ENOMEM;
 
 	pages = obj->base.size / PAGE_SIZE;
 
-- 
2.3.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/skl: Fix up positive error code

[Jani Nikula]

On Tue, 24 Mar 2015, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> It should have been negative since it is returned with ERR_PTR().

Please always reference the commit that introduced the issue.

commit 50470bb011c4be278097670bea92462f4e8c8945
Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Date:   Mon Mar 23 11:10:36 2015 +0000

    drm/i915/skl: Support secondary (rotated) frame buffer mapping


BR,
Jani.


>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index fc56c11..9903bb0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2540,7 +2540,7 @@ intel_rotate_fb_obj_pages(struct i915_ggtt_view *ggtt_view,
>  	struct sg_table *st;
>  	unsigned int tile_pitch, tile_height;
>  	unsigned int width_pages, height_pages;
> -	int ret = ENOMEM;
> +	int ret = -ENOMEM;
>  
>  	pages = obj->base.size / PAGE_SIZE;
>  
> -- 
> 2.3.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/skl: Fix up positive error code

[Tvrtko Ursulin]

On 03/24/2015 01:16 PM, Jani Nikula wrote:
> On Tue, 24 Mar 2015, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> It should have been negative since it is returned with ERR_PTR().
>
> Please always reference the commit that introduced the issue.

Is there some more precisely defined criteria for "always"?

In this case this is at the moment dead code freshly merged to -nightly 
only so ideally it would even be better to fix it up in the original 
patch if possible?

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/skl: Fix up positive error code

[Jani Nikula]

On Tue, 24 Mar 2015, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> On 03/24/2015 01:16 PM, Jani Nikula wrote:
>> On Tue, 24 Mar 2015, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> It should have been negative since it is returned with ERR_PTR().
>>
>> Please always reference the commit that introduced the issue.
>
> Is there some more precisely defined criteria for "always"?

Always when you fix a bug that was introduced by another commit?

I need the commit reference to check if I need to queue the fix to
current development kernel (i.e. v4.0-rcN).

If someone backports the commit, it's easier to check if the commit is
referenced by a later commit potentially fixing issues in it.

> In this case this is at the moment dead code freshly merged to -nightly 
> only so ideally it would even be better to fix it up in the original 
> patch if possible?

I typically wouldn't know this by looking at a patch. The commit
reference helps. And to squash it into another commit, the reference
helps.

Reducing the burden from the maintainers helps you too in the end!

BR,
Jani.


>
> Regards,
>
> Tvrtko

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/skl: Fix up positive error code

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6037
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -2              269/269              267/269
ILK                 -1              303/303              302/303
SNB                                  304/304              304/304
IVB                                  338/338              338/338
BYT                                  287/287              287/287
HSW                                  362/362              362/362
BDW                                  310/310              310/310
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 PNV  igt@gem_userptr_blits@coherency-sync      CRASH(1)PASS(1)      CRASH(2)
 PNV  igt@gem_userptr_blits@minor-unsync-normal      DMESG_WARN(1)PASS(1)      DMESG_WARN(1)PASS(1)
*ILK  igt@gem_seqno_wrap      PASS(2)      DMESG_WARN(1)PASS(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/skl: Fix up positive error code

[Tvrtko Ursulin]

On 03/24/2015 02:44 PM, Jani Nikula wrote:
> On Tue, 24 Mar 2015, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>> On 03/24/2015 01:16 PM, Jani Nikula wrote:
>>> On Tue, 24 Mar 2015, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> It should have been negative since it is returned with ERR_PTR().
>>>
>>> Please always reference the commit that introduced the issue.
>>
>> Is there some more precisely defined criteria for "always"?
>
> Always when you fix a bug that was introduced by another commit?
>
> I need the commit reference to check if I need to queue the fix to
> current development kernel (i.e. v4.0-rcN).
>
> If someone backports the commit, it's easier to check if the commit is
> referenced by a later commit potentially fixing issues in it.

Sure, but all bugs are introduced by other commits. So my question was 
only along those lines, nothing more than that.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2] drm/i915/skl: Fix up positive error code

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

It should have been negative since it is returned with ERR_PTR().

Introduced in new code commit:

   commit 50470bb011c4be278097670bea92462f4e8c8945
   Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
   Date:   Mon Mar 23 11:10:36 2015 +0000

    drm/i915/skl: Support secondary (rotated) frame buffer mapping

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index fc56c11..9903bb0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2540,7 +2540,7 @@ intel_rotate_fb_obj_pages(struct i915_ggtt_view *ggtt_view,
 	struct sg_table *st;
 	unsigned int tile_pitch, tile_height;
 	unsigned int width_pages, height_pages;
-	int ret = ENOMEM;
+	int ret = -ENOMEM;
 
 	pages = obj->base.size / PAGE_SIZE;
 
-- 
2.3.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/skl: Fix up positive error code

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6037
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -2              269/269              267/269
ILK                 -1              303/303              302/303
SNB                                  304/304              304/304
IVB                                  338/338              338/338
BYT                                  287/287              287/287
HSW                                  362/362              362/362
BDW                                  310/310              310/310
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 PNV  igt@gem_userptr_blits@coherency-sync      CRASH(1)PASS(3)      CRASH(2)
 PNV  igt@gem_userptr_blits@minor-unsync-normal      DMESG_WARN(1)PASS(1)      DMESG_WARN(1)PASS(1)
*ILK  igt@gem_seqno_wrap      PASS(3)      DMESG_WARN(1)PASS(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/skl: Fix up positive error code

[Daniel Vetter]

On Tue, Mar 24, 2015 at 05:21:35PM +0000, Tvrtko Ursulin wrote:
> On 03/24/2015 02:44 PM, Jani Nikula wrote:
> >On Tue, 24 Mar 2015, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> >>On 03/24/2015 01:16 PM, Jani Nikula wrote:
> >>>On Tue, 24 Mar 2015, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> >>>>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>
> >>>>It should have been negative since it is returned with ERR_PTR().
> >>>
> >>>Please always reference the commit that introduced the issue.
> >>
> >>Is there some more precisely defined criteria for "always"?
> >
> >Always when you fix a bug that was introduced by another commit?
> >
> >I need the commit reference to check if I need to queue the fix to
> >current development kernel (i.e. v4.0-rcN).
> >
> >If someone backports the commit, it's easier to check if the commit is
> >referenced by a later commit potentially fixing issues in it.
> 
> Sure, but all bugs are introduced by other commits. So my question was only
> along those lines, nothing more than that.

In that sense it's an always always. If you don't maintainers have to do
this, and that usually means more errors and screwups. Jani&I are bringing
this up since we're just going through some fireworks because of this.

For similar reasons you should also always cc the author/reviewers of the
offending commit. Not a lot of people read all of intel-gfx, cc'ing
relevant people is therefore important to keep everyone in the loop.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/skl: Fix up positive error code

[Daniel Vetter]

On Wed, Mar 25, 2015 at 10:15:26AM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> It should have been negative since it is returned with ERR_PTR().
> 
> Introduced in new code commit:
> 
>    commit 50470bb011c4be278097670bea92462f4e8c8945
>    Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>    Date:   Mon Mar 23 11:10:36 2015 +0000
> 
>     drm/i915/skl: Support secondary (rotated) frame buffer mapping
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

Queued for -next, thanks for the patch.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index fc56c11..9903bb0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2540,7 +2540,7 @@ intel_rotate_fb_obj_pages(struct i915_ggtt_view *ggtt_view,
>  	struct sg_table *st;
>  	unsigned int tile_pitch, tile_height;
>  	unsigned int width_pages, height_pages;
> -	int ret = ENOMEM;
> +	int ret = -ENOMEM;
>  
>  	pages = obj->base.size / PAGE_SIZE;
>  
> -- 
> 2.3.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx