Message ID | 1385484699-51596-7-git-send-email-bradley.d.volkin@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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
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
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
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 --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;