[Intel-gfx] [PATCH] drm/i915: Add new GET_PIPE_FROM_CRTC_ID ioctl.

[Julien Cristau]

Hi Carl,

I'm a novice at kernel stuff, so take this with a grain of salt...

On Wed, Apr 29, 2009 at 14:43:54 -0700, Carl Worth wrote:

> This allows userlevel code to discover the pipe number corresponding
> to a given CRTC ID. This is necessary for doing pipe-specific
> operations such as waiting for vblank on a given CRTC.
> ---
> 
>  I've got a couple of style questions here:
> 
>  * I gave the new function an i915_ prefix like all the other ioctls,
>    but it ends up as i915_get_pipe_from_crtc_id right next to
>    intel_get_crtc_from_pipe, so should I actually give it an intel_
>    prefix instead? I can't be consistent with everything here.
> 
>  * Documentation/CodingStyle suggests using only tabs for indentation,
>    but I notice other functions using tabs and spaces to nicely align
>    long function signatures that are broken into multiple lines. Should
>    I do that here as well?
> 
Use tabs for indentation, space for alignment, I think.

>  drivers/gpu/drm/i915/i915_dma.c      |    1 +
>  drivers/gpu/drm/i915/intel_display.c |   29 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |    2 ++
>  include/drm/i915_drm.h               |   10 ++++++++++
>  4 files changed, 42 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 051134c..77c218d 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1350,6 +1350,7 @@ struct drm_ioctl_desc i915_ioctls[] = {
>  	DRM_IOCTL_DEF(DRM_I915_GEM_SET_TILING, i915_gem_set_tiling, 0),
>  	DRM_IOCTL_DEF(DRM_I915_GEM_GET_TILING, i915_gem_get_tiling, 0),
>  	DRM_IOCTL_DEF(DRM_I915_GEM_GET_APERTURE, i915_gem_get_aperture_ioctl, 0),
> +	DRM_IOCTL_DEF(DRM_I915_GET_PIPE_FROM_CRTC_ID, i915_get_pipe_from_crtc_id, 0),
>  };
>  
>  int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index bdcda36..39551c4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1804,6 +1804,35 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  	}
>  }
>  
> +int i915_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
> +			struct drm_file *file_priv)
> +{
> +	drm_i915_private_t *dev_priv = dev->dev_private;
> +	struct drm_i915_get_pipe_from_crtc_id *pipe_from_crtc_id = data;
> +	struct drm_crtc *crtc = NULL;
> +	int pipe = -1;
> +
> +	if (!dev_priv) {
> +		DRM_ERROR("called with no initialization\n");
> +		return -EINVAL;
> +	}
> +
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> +		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +		if (crtc->base.id == pipe_from_crtc_id->crtc_id)
> +			pipe = intel_crtc->pipe;

break; here?

> +	}
> +
> +	if (pipe == -1 ) {
> +		DRM_ERROR("no such CRTC id\n");
> +		return -EINVAL;
> +       }
> +

does anything ensure that these DRM_ERRORs can't be trigerred by any
user process?

> +	pipe_from_crtc_id->pipe = pipe;
> +
> +       return 0;
> +}
> +
>  struct drm_crtc *intel_get_crtc_from_pipe(struct drm_device *dev, int pipe)
>  {
>  	struct drm_crtc *crtc = NULL;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 957daef..2238632 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -125,6 +125,8 @@ extern struct drm_encoder *intel_best_encoder(struct drm_connector *connector);
>  
>  extern struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
>  						    struct drm_crtc *crtc);
> +int i915_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
> +			struct drm_file *file_priv);

missing extern here.

>  extern void intel_wait_for_vblank(struct drm_device *dev);
>  extern struct drm_crtc *intel_get_crtc_from_pipe(struct drm_device *dev, int pipe);
>  extern struct drm_crtc *intel_get_load_detect_pipe(struct intel_output *intel_output,
> diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
> index 95962fa..0508a6e 100644
> --- a/include/drm/i915_drm.h
> +++ b/include/drm/i915_drm.h
> @@ -184,6 +184,7 @@ typedef struct _drm_i915_sarea {
>  #define DRM_I915_GEM_GET_TILING	0x22
>  #define DRM_I915_GEM_GET_APERTURE 0x23
>  #define DRM_I915_GEM_MMAP_GTT	0x24
> +#define DRM_I915_GET_PIPE_FROM_CRTC_ID	0x25
>  
>  #define DRM_IOCTL_I915_INIT		DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
>  #define DRM_IOCTL_I915_FLUSH		DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH)
> @@ -219,6 +220,7 @@ typedef struct _drm_i915_sarea {
>  #define DRM_IOCTL_I915_GEM_SET_TILING	DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_SET_TILING, struct drm_i915_gem_set_tiling)
>  #define DRM_IOCTL_I915_GEM_GET_TILING	DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_GET_TILING, struct drm_i915_gem_get_tiling)
>  #define DRM_IOCTL_I915_GEM_GET_APERTURE	DRM_IOR  (DRM_COMMAND_BASE + DRM_I915_GEM_GET_APERTURE, struct drm_i915_gem_get_aperture)
> +#define DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GET_PIPE_FROM_CRTC_ID, struct drm_i915_get_pipe_from_crtc_id)
>  
>  /* Allow drivers to submit batchbuffers directly to hardware, relying
>   * on the security mechanisms provided by hardware.
> @@ -657,4 +659,12 @@ struct drm_i915_gem_get_aperture {
>  	__u64 aper_available_size;
>  };
>  
> +struct drm_i915_get_pipe_from_crtc_id {
> +	/** ID of CRTC being requested **/
> +	__u32 crtc_id;
> +
> +	/** pipe of requested CRTC **/
> +	__u32 pipe;
> +};
> +
>  #endif				/* _I915_DRM_H_ */

Cheers,
Julien