From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sagar Arun Kamble Subject: Re: [RFC 1/1] drm/i915: Added support for setting plane alpha through drm property Date: Mon, 24 Feb 2014 21:14:41 +0530 Message-ID: <1393256681.31978.0.camel@sagar-desktop> References: <1392804488-5551-1-git-send-email-sagar.a.kamble@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 671A6FAB05 for ; Mon, 24 Feb 2014 07:43:43 -0800 (PST) In-Reply-To: <1392804488-5551-1-git-send-email-sagar.a.kamble@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org Gentle Reminder !!! Kindly review and provide feedback thanks, Sagar On Wed, 2014-02-19 at 15:38 +0530, sagar.a.kamble@intel.com wrote: > From: Sagar Kamble > > With this patch two properties are added. One for CRTC+Sprite planes > and another for Cursor planes. Through these client will be able to > change the pixel format of the planes w.r.t Alpha channel. > Number of drm properties are limited so should we restrain from adding this > as drm property or should we expose this as IOCTL itself. > > Signed-off-by: Sagar Kamble > --- > drivers/gpu/drm/i915/i915_drv.h | 2 + > drivers/gpu/drm/i915/i915_reg.h | 6 ++ > drivers/gpu/drm/i915/intel_display.c | 156 +++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 8 +- > drivers/gpu/drm/i915/intel_sprite.c | 59 +++++++++++-- > 5 files changed, 222 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 8c64831..5ced53d 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1560,6 +1560,8 @@ typedef struct drm_i915_private { > > struct drm_property *broadcast_rgb_property; > struct drm_property *force_audio_property; > + struct drm_property *plane_alpha_property; > + struct drm_property *cursor_alpha_property; > > uint32_t hw_context_size; > struct list_head context_list; > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 2f564ce..039270e 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -3522,7 +3522,11 @@ > /* New style CUR*CNTR flags */ > #define CURSOR_MODE 0x27 > #define CURSOR_MODE_DISABLE 0x00 > +#define CURSOR_MODE_128_32B_AX 0x02 > +#define CURSOR_MODE_256_32B_AX 0x03 > #define CURSOR_MODE_64_32B_AX 0x07 > +#define CURSOR_MODE_128_ARGB_AX ((1 << 5) | CURSOR_MODE_128_32B_AX) > +#define CURSOR_MODE_256_ARGB_AX ((1 << 5) | CURSOR_MODE_256_32B_AX) > #define CURSOR_MODE_64_ARGB_AX ((1 << 5) | CURSOR_MODE_64_32B_AX) > #define MCURSOR_PIPE_SELECT (1 << 28) > #define MCURSOR_PIPE_A 0x00 > @@ -3569,7 +3573,9 @@ > #define DISPPLANE_RGBX101010 (0x8<<26) > #define DISPPLANE_RGBA101010 (0x9<<26) > #define DISPPLANE_BGRX101010 (0xa<<26) > +#define DISPPLANE_BGRA101010 (0xb<<26) > #define DISPPLANE_RGBX161616 (0xc<<26) > +#define DISPPLANE_RGBA161616 (0xd<<26) > #define DISPPLANE_RGBX888 (0xe<<26) > #define DISPPLANE_RGBA888 (0xf<<26) > #define DISPPLANE_STEREO_ENABLE (1<<25) > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index f19e6ea..a854bcb 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2047,6 +2047,86 @@ unsigned long intel_gen4_compute_page_offset(int *x, int *y, > } > } > > +u32 control_plane_alpha(u32 pixformat, unsigned int alpha) > +{ > + switch (pixformat) { > + case DISPPLANE_RGBX888: > + case DISPPLANE_RGBA888: > + if (alpha) > + pixformat = DISPPLANE_RGBA888; > + else > + pixformat = DISPPLANE_RGBX888; > + break; > + case DISPPLANE_BGRX888: > + case DISPPLANE_BGRA888: > + if (alpha) > + pixformat = DISPPLANE_BGRA888; > + else > + pixformat = DISPPLANE_BGRX888; > + break; > + case DISPPLANE_RGBX101010: > + case DISPPLANE_RGBA101010: > + if (alpha) > + pixformat = DISPPLANE_RGBA101010; > + else > + pixformat = DISPPLANE_RGBX101010; > + break; > + case DISPPLANE_BGRX101010: > + case DISPPLANE_BGRA101010: > + if (alpha) > + pixformat = DISPPLANE_BGRA101010; > + else > + pixformat = DISPPLANE_BGRX101010; > + break; > + case DISPPLANE_RGBX161616: > + case DISPPLANE_RGBA161616: > + if (alpha) > + pixformat = DISPPLANE_RGBA161616; > + else > + pixformat = DISPPLANE_RGBX161616; > + break; > + default: > + if (alpha) > + DRM_DEBUG_DRIVER("Pixel format 0x%08x does not support Alpha Control\n", pixformat); > + break; > + } > + return pixformat; > +} > + > +u32 control_cursor_alpha(u32 pixformat, unsigned int alpha) > +{ > + switch (pixformat) { > + case CURSOR_MODE_128_32B_AX: > + case CURSOR_MODE_128_ARGB_AX: > + if (alpha) > + pixformat = CURSOR_MODE_128_ARGB_AX; > + else > + pixformat = CURSOR_MODE_128_32B_AX; > + break; > + > + case CURSOR_MODE_256_ARGB_AX: > + case CURSOR_MODE_256_32B_AX: > + if (alpha) > + pixformat = CURSOR_MODE_256_ARGB_AX; > + else > + pixformat = CURSOR_MODE_256_32B_AX; > + break; > + > + case CURSOR_MODE_64_ARGB_AX: > + case CURSOR_MODE_64_32B_AX: > + if (alpha) > + pixformat = CURSOR_MODE_64_ARGB_AX; > + else > + pixformat = CURSOR_MODE_64_32B_AX; > + break; > + default: > + if (alpha) > + DRM_DEBUG_DRIVER("Pixel format 0x%08x does not support Alpha Control\n", pixformat); > + break; > + } > + return pixformat; > +} > + > static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb, > int x, int y) > { > @@ -2107,6 +2187,9 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb, > BUG(); > } > > + dspcntr |= control_plane_alpha(dspcntr & DISPPLANE_PIXFORMAT_MASK, > + intel_crtc->primary_alpha); > + > if (INTEL_INFO(dev)->gen >= 4) { > if (obj->tiling_mode != I915_TILING_NONE) > dspcntr |= DISPPLANE_TILED; > @@ -7415,6 +7498,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base) > if (base) { > cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT); > cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE; > + cntl |= control_cursor_alpha(cntl & CURSOR_MODE, intel_crtc->cursor_alpha); > cntl |= pipe << 28; /* Connect to correct pipe */ > } else { > cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE); > @@ -8755,6 +8839,59 @@ free_work: > return ret; > } > > +static int intel_set_primary_plane_alpha(struct intel_crtc *crtc, > + unsigned int alpha) > +{ > + struct drm_device *dev = crtc->base.dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + unsigned int old_alpha; > + int ret = 0; > + > + old_alpha = crtc->primary_alpha; > + crtc->primary_alpha = alpha; > + > + if (!crtc->active) > + return 0; > + > + intel_crtc_wait_for_pending_flips(&crtc->base); > + > + ret = dev_priv->display.update_plane(&crtc->base, crtc->base.fb, 0, 0); > + if (ret) > + crtc->primary_alpha = old_alpha; > + > + return ret; > +} > + > +static void intel_set_cursor_plane_alpha(struct intel_crtc *crtc, > + unsigned int alpha) > +{ > + crtc->cursor_alpha = alpha; > + > + if (crtc->active) > + intel_crtc_update_cursor(&crtc->base, true); > +} > + > +static int intel_crtc_set_property(struct drm_crtc *crtc, > + struct drm_property *prop, > + uint64_t val) > +{ > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + uint64_t old_val; > + int ret = -ENOENT; > + > + if (prop == dev_priv->plane_alpha_property) { > + ret = intel_set_primary_plane_alpha(intel_crtc, > + intel_crtc->primary_alpha); > + } else if (prop == dev_priv->cursor_alpha_property) { > + intel_set_cursor_plane_alpha(intel_crtc, val); > + return 0; > + } > + > + return ret; > +} > + > static struct drm_crtc_helper_funcs intel_helper_funcs = { > .mode_set_base_atomic = intel_pipe_set_base_atomic, > .load_lut = intel_crtc_load_lut, > @@ -10167,6 +10304,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = { > .set_config = intel_crtc_set_config, > .destroy = intel_crtc_destroy, > .page_flip = intel_crtc_page_flip, > + .set_property = intel_crtc_set_property > }; > > static void intel_cpu_pll_init(struct drm_device *dev) > @@ -10305,6 +10443,24 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) > dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base; > dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = &intel_crtc->base; > > + if (!dev_priv->plane_alpha_property) > + dev_priv->plane_alpha_property = > + drm_property_create_range(dev, 0, "plane-alpha", 0, 1); > + > + if (dev_priv->plane_alpha_property) > + drm_object_attach_property(&intel_crtc->base.base, > + dev_priv->plane_alpha_property, > + intel_crtc->primary_alpha); > + > + if (!dev_priv->cursor_alpha_property) > + dev_priv->cursor_alpha_property = > + drm_property_create_range(dev, 0, "cursor-alpha", 0, 1); > + > + if (dev_priv->cursor_alpha_property) > + drm_object_attach_property(&intel_crtc->base.base, > + dev_priv->cursor_alpha_property, > + intel_crtc->cursor_alpha); > + > drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs); > } > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index a4ffc02..0080d3a 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -339,6 +339,9 @@ struct intel_crtc { > struct drm_crtc base; > enum pipe pipe; > enum plane plane; > + unsigned int primary_alpha; /* primary plane alpha control */ > + unsigned int cursor_alpha; /* cursor plane alpha control */ > + > u8 lut_r[256], lut_g[256], lut_b[256]; > /* > * Whether the crtc and the connected output pipeline is active. Implies > @@ -405,6 +408,7 @@ struct intel_plane { > unsigned int crtc_w, crtc_h; > uint32_t src_x, src_y; > uint32_t src_w, src_h; > + unsigned int plane_alpha; > > /* Since we need to change the watermarks before/after > * enabling/disabling the planes, we need to store the parameters here > @@ -676,6 +680,8 @@ int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data, > enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv, > enum pipe pipe); > void intel_wait_for_vblank(struct drm_device *dev, int pipe); > +u32 control_plane_alpha(u32 pixformat, unsigned int alpha); > +u32 control_cursor_alpha(u32 pixformat, unsigned int alpha); > void intel_wait_for_pipe_off(struct drm_device *dev, int pipe); > int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp); > void vlv_wait_port_ready(struct drm_i915_private *dev_priv, > @@ -906,7 +912,7 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob); > int intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane); > void intel_flush_primary_plane(struct drm_i915_private *dev_priv, > enum plane plane); > -void intel_plane_restore(struct drm_plane *plane); > +int intel_plane_restore(struct drm_plane *plane); > void intel_plane_disable(struct drm_plane *plane); > int intel_sprite_set_colorkey(struct drm_device *dev, void *data, > struct drm_file *file_priv); > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 336ae6c..9f6c91a 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -104,6 +104,9 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc, > break; > } > > + sprctl |= control_plane_alpha(sprctl & SP_PIXFORMAT_MASK, > + intel_plane->plane_alpha); > + > /* > * Enable gamma to match primary/cursor plane behaviour. > * FIXME should be user controllable via propertiesa. > @@ -262,6 +265,9 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > BUG(); > } > > + sprctl |= control_plane_alpha(sprctl & SPRITE_PIXFORMAT_MASK, > + intel_plane->plane_alpha); > + > /* > * Enable gamma to match primary/cursor plane behaviour. > * FIXME should be user controllable via propertiesa. > @@ -446,6 +452,9 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > BUG(); > } > > + dvscntr |= control_plane_alpha(dvscntr & DVS_PIXFORMAT_MASK, > + intel_plane->plane_alpha); > + > /* > * Enable gamma to match primary/cursor plane behaviour. > * FIXME should be user controllable via propertiesa. > @@ -1011,18 +1020,38 @@ out_unlock: > return ret; > } > > -void intel_plane_restore(struct drm_plane *plane) > +static int intel_plane_set_property(struct drm_plane *plane, > + struct drm_property *prop, > + uint64_t val) > +{ > + struct drm_i915_private *dev_priv = plane->dev->dev_private; > + struct intel_plane *intel_plane = to_intel_plane(plane); > + uint64_t old_val; > + int ret = -ENOENT; > + > + if (prop == dev_priv->plane_alpha_property) { > + old_val = intel_plane->plane_alpha; > + intel_plane->plane_alpha = val; > + ret = intel_plane_restore(plane); > + if (ret) > + intel_plane->plane_alpha = old_val; > + } > + > + return ret; > +} > + > +int intel_plane_restore(struct drm_plane *plane) > { > struct intel_plane *intel_plane = to_intel_plane(plane); > > if (!plane->crtc || !plane->fb) > - return; > + return 0; > > - intel_update_plane(plane, plane->crtc, plane->fb, > - intel_plane->crtc_x, intel_plane->crtc_y, > - intel_plane->crtc_w, intel_plane->crtc_h, > - intel_plane->src_x, intel_plane->src_y, > - intel_plane->src_w, intel_plane->src_h); > + return intel_update_plane(plane, plane->crtc, plane->fb, > + intel_plane->crtc_x, intel_plane->crtc_y, > + intel_plane->crtc_w, intel_plane->crtc_h, > + intel_plane->src_x, intel_plane->src_y, > + intel_plane->src_w, intel_plane->src_h); > } > > void intel_plane_disable(struct drm_plane *plane) > @@ -1037,6 +1066,7 @@ static const struct drm_plane_funcs intel_plane_funcs = { > .update_plane = intel_update_plane, > .disable_plane = intel_disable_plane, > .destroy = intel_destroy_plane, > + .set_property = intel_plane_set_property, > }; > > static uint32_t ilk_plane_formats[] = { > @@ -1073,6 +1103,7 @@ static uint32_t vlv_plane_formats[] = { > int > intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) > { > + struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_plane *intel_plane; > unsigned long possible_crtcs; > const uint32_t *plane_formats; > @@ -1146,8 +1177,20 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) > &intel_plane_funcs, > plane_formats, num_plane_formats, > false); > - if (ret) > + if (ret) { > kfree(intel_plane); > + goto out; > + } > + > + if (!dev_priv->plane_alpha_property) > + dev_priv->plane_alpha_property = > + drm_property_create_range(dev, 0, "plane-alpha", 0, 1); > + > + if (dev_priv->plane_alpha_property) > + drm_object_attach_property(&intel_plane->base.base, > + dev_priv->plane_alpha_property, > + intel_plane->plane_alpha); > > +out: > return ret; > }