[v6,15/19] drm/i915: batch_obj vm offset must be u64

Submitted by Michel Thierry on July 29, 2015, 4:23 p.m.

Details

Message ID 1438187043-34267-16-git-send-email-michel.thierry@intel.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Michel Thierry July 29, 2015, 4:23 p.m.
Otherwise it can overflow in 48-bit mode, and cause an incorrect
exec_start.

Before commit 5f19e2bffa63a91cd4ac1adcec648e14a44277ce ("drm/i915: Merged
the many do_execbuf() parameters into a structure"), it was already an u64.

Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 33926d9..ed2fbcd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1674,7 +1674,7 @@  struct i915_execbuffer_params {
 	struct drm_file                 *file;
 	uint32_t                        dispatch_flags;
 	uint32_t                        args_batch_start_offset;
-	uint32_t                        batch_obj_vm_offset;
+	uint64_t                        batch_obj_vm_offset;
 	struct intel_engine_cs          *ring;
 	struct drm_i915_gem_object      *batch_obj;
 	struct intel_context            *ctx;

Comments

Reviewed the patch & it looks fine.
Reviewed-by: "Akash Goel <akash.goel@intel.com>"

On 7/29/2015 9:53 PM, Michel Thierry wrote:
> Otherwise it can overflow in 48-bit mode, and cause an incorrect
> exec_start.
>
> Before commit 5f19e2bffa63a91cd4ac1adcec648e14a44277ce ("drm/i915: Merged
> the many do_execbuf() parameters into a structure"), it was already an u64.
>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 33926d9..ed2fbcd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1674,7 +1674,7 @@ struct i915_execbuffer_params {
>   	struct drm_file                 *file;
>   	uint32_t                        dispatch_flags;
>   	uint32_t                        args_batch_start_offset;
> -	uint32_t                        batch_obj_vm_offset;
> +	uint64_t                        batch_obj_vm_offset;
>   	struct intel_engine_cs          *ring;
>   	struct drm_i915_gem_object      *batch_obj;
>   	struct intel_context            *ctx;
>
On Wed, Jul 29, 2015 at 05:23:59PM +0100, Michel Thierry wrote:
> Otherwise it can overflow in 48-bit mode, and cause an incorrect
> exec_start.
> 
> Before commit 5f19e2bffa63a91cd4ac1adcec648e14a44277ce ("drm/i915: Merged
> the many do_execbuf() parameters into a structure"), it was already an u64.
> 
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>

So we have a few more u64, but the i915_gem_obj_offset is still unsigned
long. Am I missing a patch?
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 33926d9..ed2fbcd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1674,7 +1674,7 @@ struct i915_execbuffer_params {
>  	struct drm_file                 *file;
>  	uint32_t                        dispatch_flags;
>  	uint32_t                        args_batch_start_offset;
> -	uint32_t                        batch_obj_vm_offset;
> +	uint64_t                        batch_obj_vm_offset;
>  	struct intel_engine_cs          *ring;
>  	struct drm_i915_gem_object      *batch_obj;
>  	struct intel_context            *ctx;
> -- 
> 2.4.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 8/5/2015 5:01 PM, Daniel Vetter wrote:
> On Wed, Jul 29, 2015 at 05:23:59PM +0100, Michel Thierry wrote:
>> Otherwise it can overflow in 48-bit mode, and cause an incorrect
>> exec_start.
>>
>> Before commit 5f19e2bffa63a91cd4ac1adcec648e14a44277ce ("drm/i915: Merged
>> the many do_execbuf() parameters into a structure"), it was already an u64.
>>
>> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
>
> So we have a few more u64, but the i915_gem_obj_offset is still unsigned
> long. Am I missing a patch?

http://news.gmane.org/find-root.php?message_id=1437063498-31930-1-git-send-email-michel.thierry@intel.com

Which I need to re-send with the comments I got.
Thanks for remind me.
On Wed, Aug 05, 2015 at 05:14:03PM +0100, Michel Thierry wrote:
> On 8/5/2015 5:01 PM, Daniel Vetter wrote:
> >On Wed, Jul 29, 2015 at 05:23:59PM +0100, Michel Thierry wrote:
> >>Otherwise it can overflow in 48-bit mode, and cause an incorrect
> >>exec_start.
> >>
> >>Before commit 5f19e2bffa63a91cd4ac1adcec648e14a44277ce ("drm/i915: Merged
> >>the many do_execbuf() parameters into a structure"), it was already an u64.
> >>
> >>Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> >
> >So we have a few more u64, but the i915_gem_obj_offset is still unsigned
> >long. Am I missing a patch?
> 
> http://news.gmane.org/find-root.php?message_id=1437063498-31930-1-git-send-email-michel.thierry@intel.com
> 
> Which I need to re-send with the comments I got.
> Thanks for remind me.

Process reminder: If your patch series has depencies either
- include them at the start (git will correctly keep authorship), which is
  the preferred approach
- or at least mention your depencies in the cover letter

Relying on your maintainer's mind-reader to figure this out doesn't scale ;-)

Cheers, Daniel