Re: [PATCH 3/3] intel: Add a function for the new register read ioctl.

[Ben Widawsky <ben@bwidawsk.net>]

On 2012-08-02 11:29, Eric Anholt wrote:
> ---
> I'm not sure if this is the API we want or not.  Getting your value
> back through an out parameter sucks.  On the other hand, the 3d 
> driver
> wants to be able to use the error value to detect a supported kernel.
>
> My other API thought would be for reg_read() to return the value and
> whine if an error occurs, and to have a separate can_reg_read() call.

I'd like reg_read to just do what we want. I think making ugly code for
backward compatibility is less than ideal. OTOH, mesa and whomever else
could just wrap this in a function that does what they want. 
Furthermore,
we can just supplement a macro to also do what we want. So I think this
approach is appropriate.

Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

>
>  intel/intel_bufmgr.h     |    3 +++
>  intel/intel_bufmgr_gem.c |   18 ++++++++++++++++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h
> index 2167e43..8d7f239 100644
> --- a/intel/intel_bufmgr.h
> +++ b/intel/intel_bufmgr.h
> @@ -241,6 +241,9 @@ void drm_intel_decode_set_head_tail(struct
> drm_intel_decode *ctx,
>  void drm_intel_decode_set_output_file(struct drm_intel_decode *ctx,
> FILE *out);
>  void drm_intel_decode(struct drm_intel_decode *ctx);
>
> +int drm_intel_reg_read(drm_intel_bufmgr *bufmgr,
> +		       uint32_t offset,
> +		       uint64_t *result);
>
>  /** @{ Compatibility defines to keep old code building despite the
> symbol rename
>   * from dri_* to drm_intel_*
> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> index a484b12..a170813 100644
> --- a/intel/intel_bufmgr_gem.c
> +++ b/intel/intel_bufmgr_gem.c
> @@ -2947,6 +2947,24 @@ 
> drm_intel_gem_context_destroy(drm_intel_context *ctx)
>  	free(ctx);
>  }
>
> +int
> +drm_intel_reg_read(drm_intel_bufmgr *bufmgr,
> +		   uint32_t offset,
> +		   uint64_t *result)
> +{
> +	drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bufmgr;
> +	struct drm_i915_reg_read reg_read;
> +	int ret;
> +
> +	VG_CLEAR(reg_read);
> +	reg_read.offset = offset;
> +
> +	ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_REG_READ, &reg_read);
> +
> +	*result = reg_read.val;
> +	return ret;
> +}
> +
>
>  /**
>   * Annotate the given bo for use in aub dumping.