diff mbox

[RFC,05/21] drm/i915: Add helper functions to aid seqno -> request transition

Message ID 1412604925-11290-6-git-send-email-John.C.Harrison@Intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

John Harrison Oct. 6, 2014, 2:15 p.m. UTC 
From: John Harrison <John.C.Harrison@Intel.com>

For: VIZ-4377
Signed-off-by: John.C.Harrison@Intel.com
---
 drivers/gpu/drm/i915/i915_drv.h         |   12 ++++++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.h |    7 +++++++
 2 files changed, 19 insertions(+)

Comments

Daniel Vetter Oct. 19, 2014, 12:35 p.m. UTC | #1 
On Mon, Oct 06, 2014 at 03:15:09PM +0100, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 

I think a few words about how these helpers help the transitions (and
example maybe) would be great. Since without peeking ahead (which I
didn't) and looking at the JIRA (which I've forgotten all about already
right after writing it) this doesn't make a lot of sense ;-)

> For: VIZ-4377
> Signed-off-by: John.C.Harrison@Intel.com
> ---
>  drivers/gpu/drm/i915/i915_drv.h         |   12 ++++++++++++
>  drivers/gpu/drm/i915/intel_ringbuffer.h |    7 +++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e1858e7..62c9f66 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1946,6 +1946,18 @@ struct drm_i915_gem_request {
>  
>  void i915_gem_request_free(struct kref *req_ref);
>  
> +static inline uint32_t
> +i915_gem_request_get_seqno(struct drm_i915_gem_request *req)
> +{
> +	return req ? req->seqno : 0;
> +}
> +
> +static inline struct intel_engine_cs *
> +i915_gem_request_get_ring(struct drm_i915_gem_request *req)
> +{
> +	return req ? req->ring : NULL;
> +}
> +
>  static inline void
>  i915_gem_request_reference(struct drm_i915_gem_request *req)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 96479c8..cc1b62f 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -435,6 +435,13 @@ static inline u32 intel_ring_get_seqno(struct intel_engine_cs *ring)
>  	return ring->outstanding_lazy_seqno;
>  }
>  
> +static inline struct drm_i915_gem_request *
> +intel_ring_get_request(struct intel_engine_cs *ring)
> +{
> +	BUG_ON(ring->preallocated_lazy_request == NULL);

Again, I'm not a fan of BUG_ON. But if this is just a transitional thing
and will disappear at the end of the series I'm ok with it.
-Daniel

> +	return ring->preallocated_lazy_request;
> +}
> +
>  static inline void i915_trace_irq_get(struct intel_engine_cs *ring, u32 seqno)
>  {
>  	if (ring->trace_irq_seqno == 0 && ring->irq_get(ring))
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
John Harrison Oct. 20, 2014, 2:49 p.m. UTC | #2 
On 19/10/2014 13:35, Daniel Vetter wrote:
> On Mon, Oct 06, 2014 at 03:15:09PM +0100, John.C.Harrison@Intel.com wrote:
>> From: John Harrison <John.C.Harrison@Intel.com>
>>
> I think a few words about how these helpers help the transitions (and
> example maybe) would be great. Since without peeking ahead (which I
> didn't) and looking at the JIRA (which I've forgotten all about already
> right after writing it) this doesn't make a lot of sense ;-)
>
>> For: VIZ-4377
>> Signed-off-by: John.C.Harrison@Intel.com
>> ---
>>   drivers/gpu/drm/i915/i915_drv.h         |   12 ++++++++++++
>>   drivers/gpu/drm/i915/intel_ringbuffer.h |    7 +++++++
>>   2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index e1858e7..62c9f66 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1946,6 +1946,18 @@ struct drm_i915_gem_request {
>>   
>>   void i915_gem_request_free(struct kref *req_ref);
>>   
>> +static inline uint32_t
>> +i915_gem_request_get_seqno(struct drm_i915_gem_request *req)
>> +{
>> +	return req ? req->seqno : 0;
>> +}
>> +
>> +static inline struct intel_engine_cs *
>> +i915_gem_request_get_ring(struct drm_i915_gem_request *req)
>> +{
>> +	return req ? req->ring : NULL;
>> +}
>> +
>>   static inline void
>>   i915_gem_request_reference(struct drm_i915_gem_request *req)
>>   {
>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
>> index 96479c8..cc1b62f 100644
>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
>> @@ -435,6 +435,13 @@ static inline u32 intel_ring_get_seqno(struct intel_engine_cs *ring)
>>   	return ring->outstanding_lazy_seqno;
>>   }
>>   
>> +static inline struct drm_i915_gem_request *
>> +intel_ring_get_request(struct intel_engine_cs *ring)
>> +{
>> +	BUG_ON(ring->preallocated_lazy_request == NULL);
> Again, I'm not a fan of BUG_ON. But if this is just a transitional thing
> and will disappear at the end of the series I'm ok with it.
> -Daniel
I just copied the existing 'intel_ring_get_seqno()' function and 
replaced 'seqno' with 'request'. The outgoing existing code has a BUG_ON 
therefore it seemed sensible to include a BUG_ON in the incoming new code.

>> +	return ring->preallocated_lazy_request;
>> +}
>> +
>>   static inline void i915_trace_irq_get(struct intel_engine_cs *ring, u32 seqno)
>>   {
>>   	if (ring->trace_irq_seqno == 0 && ring->irq_get(ring))
>> -- 
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e1858e7..62c9f66 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1946,6 +1946,18 @@  struct drm_i915_gem_request {
 
 void i915_gem_request_free(struct kref *req_ref);
 
+static inline uint32_t
+i915_gem_request_get_seqno(struct drm_i915_gem_request *req)
+{
+	return req ? req->seqno : 0;
+}
+
+static inline struct intel_engine_cs *
+i915_gem_request_get_ring(struct drm_i915_gem_request *req)
+{
+	return req ? req->ring : NULL;
+}
+
 static inline void
 i915_gem_request_reference(struct drm_i915_gem_request *req)
 {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 96479c8..cc1b62f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -435,6 +435,13 @@  static inline u32 intel_ring_get_seqno(struct intel_engine_cs *ring)
 	return ring->outstanding_lazy_seqno;
 }
 
+static inline struct drm_i915_gem_request *
+intel_ring_get_request(struct intel_engine_cs *ring)
+{
+	BUG_ON(ring->preallocated_lazy_request == NULL);
+	return ring->preallocated_lazy_request;
+}
+
 static inline void i915_trace_irq_get(struct intel_engine_cs *ring, u32 seqno)
 {
 	if (ring->trace_irq_seqno == 0 && ring->irq_get(ring))