diff mbox

[RFC,06/22] drm/i915: Add a HAS_CMD_PARSER getparam

Message ID 1385484699-51596-7-git-send-email-bradley.d.volkin@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

bradley.d.volkin@intel.com Nov. 26, 2013, 4:51 p.m. UTC
From: Brad Volkin <bradley.d.volkin@intel.com>

So userspace can query the kernel for command parser support.

OTC-Tracker: AXIA-4631
Change-Id: I58af650db9f6753c2dcac9c54ab432fd31db302f
Signed-off-by: Brad Volkin <bradley.d.volkin@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 3 +++
 include/uapi/drm/i915_drm.h     | 1 +
 2 files changed, 4 insertions(+)

Comments

Daniel Vetter Nov. 27, 2013, 12:51 p.m. UTC | #1
On Tue, Nov 26, 2013 at 08:51:23AM -0800, bradley.d.volkin@intel.com wrote:
> From: Brad Volkin <bradley.d.volkin@intel.com>
> 
> So userspace can query the kernel for command parser support.
> 
> OTC-Tracker: AXIA-4631
> Change-Id: I58af650db9f6753c2dcac9c54ab432fd31db302f
> Signed-off-by: Brad Volkin <bradley.d.volkin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 3 +++
>  include/uapi/drm/i915_drm.h     | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 5aeb103..f0a4638 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1003,6 +1003,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
>  	case I915_PARAM_HAS_EXEC_HANDLE_LUT:
>  		value = 1;
>  		break;
> +	case I915_PARAM_HAS_CMD_PARSER:
> +		value = 1;

I think we need to have a CMD_PARSER_VER flag here which we can increment
every time we add new registers for additional features. Examples would be
extensions for OA, or the L3 cache control stuff the media/compute guys
want. I think we should also add a comment (maybe right here) which
explains for each version what has been added. Otherwise userspace has no
idea what kind of additional restricted commands it can submit.
-Daniel

> +		break;
>  	default:
>  		DRM_DEBUG("Unknown parameter %d\n", param->param);
>  		return -EINVAL;
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 52aed89..48cc277 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -337,6 +337,7 @@ typedef struct drm_i915_irq_wait {
>  #define I915_PARAM_HAS_EXEC_NO_RELOC	 25
>  #define I915_PARAM_HAS_EXEC_HANDLE_LUT   26
>  #define I915_PARAM_HAS_WT     	 	 27
> +#define I915_PARAM_HAS_CMD_PARSER	 28
>  
>  typedef struct drm_i915_getparam {
>  	int param;
> -- 
> 1.8.4.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Kenneth Graunke Dec. 5, 2013, 9:38 a.m. UTC | #2
On 11/27/2013 04:51 AM, Daniel Vetter wrote:
> On Tue, Nov 26, 2013 at 08:51:23AM -0800, bradley.d.volkin@intel.com wrote:
>> From: Brad Volkin <bradley.d.volkin@intel.com>
>>
>> So userspace can query the kernel for command parser support.
>>
>> OTC-Tracker: AXIA-4631
>> Change-Id: I58af650db9f6753c2dcac9c54ab432fd31db302f
>> Signed-off-by: Brad Volkin <bradley.d.volkin@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_dma.c | 3 +++
>>  include/uapi/drm/i915_drm.h     | 1 +
>>  2 files changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>> index 5aeb103..f0a4638 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -1003,6 +1003,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
>>  	case I915_PARAM_HAS_EXEC_HANDLE_LUT:
>>  		value = 1;
>>  		break;
>> +	case I915_PARAM_HAS_CMD_PARSER:
>> +		value = 1;
> 
> I think we need to have a CMD_PARSER_VER flag here which we can increment
> every time we add new registers for additional features. Examples would be
> extensions for OA, or the L3 cache control stuff the media/compute guys
> want. I think we should also add a comment (maybe right here) which
> explains for each version what has been added. Otherwise userspace has no
> idea what kind of additional restricted commands it can submit.
> -Daniel

The CMD_PARSER_VER idea makes sense to me, too.  That way userspace can
easily check whether it's allowed to do something, and we can extend
that in the future.

--Ken
bradley.d.volkin@intel.com Dec. 5, 2013, 5:22 p.m. UTC | #3
On Thu, Dec 05, 2013 at 01:38:00AM -0800, Kenneth Graunke wrote:
> On 11/27/2013 04:51 AM, Daniel Vetter wrote:
> > On Tue, Nov 26, 2013 at 08:51:23AM -0800, bradley.d.volkin@intel.com wrote:
> >> From: Brad Volkin <bradley.d.volkin@intel.com>
> >>
> >> So userspace can query the kernel for command parser support.
> >>
> >> OTC-Tracker: AXIA-4631
> >> Change-Id: I58af650db9f6753c2dcac9c54ab432fd31db302f
> >> Signed-off-by: Brad Volkin <bradley.d.volkin@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/i915_dma.c | 3 +++
> >>  include/uapi/drm/i915_drm.h     | 1 +
> >>  2 files changed, 4 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> >> index 5aeb103..f0a4638 100644
> >> --- a/drivers/gpu/drm/i915/i915_dma.c
> >> +++ b/drivers/gpu/drm/i915/i915_dma.c
> >> @@ -1003,6 +1003,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
> >>  	case I915_PARAM_HAS_EXEC_HANDLE_LUT:
> >>  		value = 1;
> >>  		break;
> >> +	case I915_PARAM_HAS_CMD_PARSER:
> >> +		value = 1;
> > 
> > I think we need to have a CMD_PARSER_VER flag here which we can increment
> > every time we add new registers for additional features. Examples would be
> > extensions for OA, or the L3 cache control stuff the media/compute guys
> > want. I think we should also add a comment (maybe right here) which
> > explains for each version what has been added. Otherwise userspace has no
> > idea what kind of additional restricted commands it can submit.
> > -Daniel
> 
> The CMD_PARSER_VER idea makes sense to me, too.  That way userspace can
> easily check whether it's allowed to do something, and we can extend
> that in the future.

Ok. Any reason to keep both HAS_CMD_PARSER and CMD_PARSER_VER? Seems like
just the version should be enough.

Brad

> 
> --Ken
Daniel Vetter Dec. 5, 2013, 5:26 p.m. UTC | #4
On Thu, Dec 5, 2013 at 6:22 PM, Volkin, Bradley D
<bradley.d.volkin@intel.com> wrote:
> Ok. Any reason to keep both HAS_CMD_PARSER and CMD_PARSER_VER? Seems like
> just the version should be enough.

Just the version should be good enough, if it's not there userspace
will get an -EINVAL. And we can just say that if the version is 0
there's no command parser. So no need for an additional flag.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 5aeb103..f0a4638 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1003,6 +1003,9 @@  static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_HAS_EXEC_HANDLE_LUT:
 		value = 1;
 		break;
+	case I915_PARAM_HAS_CMD_PARSER:
+		value = 1;
+		break;
 	default:
 		DRM_DEBUG("Unknown parameter %d\n", param->param);
 		return -EINVAL;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 52aed89..48cc277 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -337,6 +337,7 @@  typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_EXEC_NO_RELOC	 25
 #define I915_PARAM_HAS_EXEC_HANDLE_LUT   26
 #define I915_PARAM_HAS_WT     	 	 27
+#define I915_PARAM_HAS_CMD_PARSER	 28
 
 typedef struct drm_i915_getparam {
 	int param;