Op 18-06-15 om 16:21 schreef Matt Roper:
> On Mon, Jun 15, 2015 at 12:33:54PM +0200, Maarten Lankhorst wrote:
>> By making color key atomic there are no more transitional helpers.
>> The plane check function will reject the color key when a scaler is
>> active.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/intel_atomic_plane.c | 1 +
>> drivers/gpu/drm/i915/intel_display.c | 7 ++-
>> drivers/gpu/drm/i915/intel_drv.h | 6 +--
>> drivers/gpu/drm/i915/intel_sprite.c | 85 +++++++++++++++----------------
>> 4 files changed, 46 insertions(+), 53 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
>> index 91d53768df9d..10a8ecedc942 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
>> @@ -56,6 +56,7 @@ intel_create_plane_state(struct drm_plane *plane)
>>
>> state->base.plane = plane;
>> state->base.rotation = BIT(DRM_ROTATE_0);
>> + state->ckey.flags = I915_SET_COLORKEY_NONE;
>>
>> return state;
>> }
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 5facd0501a34..746c73d2ab84 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -4401,9 +4401,9 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
>> return ret;
>>
>> /* check colorkey */
>> - if (WARN_ON(intel_plane->ckey.flags != I915_SET_COLORKEY_NONE)) {
>> + if (plane_state->ckey.flags != I915_SET_COLORKEY_NONE) {
>> DRM_DEBUG_KMS("[PLANE:%d] scaling with color key not allowed",
>> - intel_plane->base.base.id);
>> + intel_plane->base.base.id);
>> return -EINVAL;
>> }
>>
>> @@ -13733,7 +13733,7 @@ intel_check_primary_plane(struct drm_plane *plane,
>>
>> /* use scaler when colorkey is not required */
>> if (INTEL_INFO(plane->dev)->gen >= 9 &&
>> - to_intel_plane(plane)->ckey.flags == I915_SET_COLORKEY_NONE) {
>> + state->ckey.flags == I915_SET_COLORKEY_NONE) {
>> min_scale = 1;
>> max_scale = skl_max_scale(to_intel_crtc(crtc), crtc_state);
>> can_position = true;
>> @@ -13881,7 +13881,6 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>> primary->check_plane = intel_check_primary_plane;
>> primary->commit_plane = intel_commit_primary_plane;
>> primary->disable_plane = intel_disable_primary_plane;
>> - primary->ckey.flags = I915_SET_COLORKEY_NONE;
>> if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
>> primary->plane = !pipe;
>>
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 93b9542ab8dc..3a2ac82b0970 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -274,6 +274,8 @@ struct intel_plane_state {
>> * update_scaler_plane.
>> */
>> int scaler_id;
>> +
>> + struct drm_intel_sprite_colorkey ckey;
>> };
>>
>> struct intel_initial_plane_config {
>> @@ -588,9 +590,6 @@ struct intel_plane {
>> bool can_scale;
>> int max_downscale;
>>
>> - /* FIXME convert to properties */
>> - struct drm_intel_sprite_colorkey ckey;
>> -
>> /* Since we need to change the watermarks before/after
>> * enabling/disabling the planes, we need to store the parameters here
>> * as the other pieces of the struct may not reflect the values we want
>> @@ -1390,7 +1389,6 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob);
>>
>> /* intel_sprite.c */
>> int intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane);
>> -int intel_plane_restore(struct drm_plane *plane);
>> int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
>> struct drm_file *file_priv);
>> bool intel_pipe_update_start(struct intel_crtc *crtc,
>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
>> index 168f90f346c2..21d3f7882c4d 100644
>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>> @@ -182,7 +182,8 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
>> const int plane = intel_plane->plane + 1;
>> u32 plane_ctl, stride_div, stride;
>> int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>> - const struct drm_intel_sprite_colorkey *key = &intel_plane->ckey;
>> + const struct drm_intel_sprite_colorkey *key =
>> + &to_intel_plane_state(drm_plane->state)->ckey;
>> unsigned long surf_addr;
>> u32 tile_height, plane_offset, plane_size;
>> unsigned int rotation;
>> @@ -344,7 +345,8 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
>> u32 sprctl;
>> unsigned long sprsurf_offset, linear_offset;
>> int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>> - const struct drm_intel_sprite_colorkey *key = &intel_plane->ckey;
>> + const struct drm_intel_sprite_colorkey *key =
>> + &to_intel_plane_state(dplane->state)->ckey;
>>
>> sprctl = SP_ENABLE;
>>
>> @@ -487,7 +489,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>> u32 sprctl, sprscale = 0;
>> unsigned long sprsurf_offset, linear_offset;
>> int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>> - const struct drm_intel_sprite_colorkey *key = &intel_plane->ckey;
>> + const struct drm_intel_sprite_colorkey *key =
>> + &to_intel_plane_state(plane->state)->ckey;
>>
>> sprctl = SPRITE_ENABLE;
>>
>> @@ -627,7 +630,8 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>> unsigned long dvssurf_offset, linear_offset;
>> u32 dvscntr, dvsscale;
>> int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>> - const struct drm_intel_sprite_colorkey *key = &intel_plane->ckey;
>> + const struct drm_intel_sprite_colorkey *key =
>> + &to_intel_plane_state(plane->state)->ckey;
>>
>> dvscntr = DVS_ENABLE;
>>
>> @@ -778,7 +782,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
>> /* setup can_scale, min_scale, max_scale */
>> if (INTEL_INFO(dev)->gen >= 9) {
>> /* use scaler when colorkey is not required */
>> - if (intel_plane->ckey.flags == I915_SET_COLORKEY_NONE) {
>> + if (state->ckey.flags == I915_SET_COLORKEY_NONE) {
>> can_scale = 1;
>> min_scale = 1;
>> max_scale = skl_max_scale(intel_crtc, crtc_state);
>> @@ -798,7 +802,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
>> * coordinates and sizes. We probably need some way to decide whether
>> * more strict checking should be done instead.
>> */
>> -
>> drm_rect_rotate(src, fb->width << 16, fb->height << 16,
>> state->base.rotation);
>>
>> @@ -808,7 +811,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
>> vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, max_scale);
>> BUG_ON(vscale < 0);
>>
>> - state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
>> + state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
>>
>> crtc_x = dst->x1;
>> crtc_y = dst->y1;
>> @@ -950,7 +953,9 @@ int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
>> {
>> struct drm_intel_sprite_colorkey *set = data;
>> struct drm_plane *plane;
>> - struct intel_plane *intel_plane;
>> + struct drm_plane_state *plane_state;
>> + struct drm_atomic_state *state;
>> + struct drm_modeset_acquire_ctx ctx;
>> int ret = 0;
> Can we simplify this by just calling
> drm_atomic_helper_plane_set_property() and let it handle the atomic
> transaction for us?
>
set_property requires drm_modeset_lock_all, and doing so requires making a specification for the
atomic color key property here. I don't know how what it would look like, so creating an atomic
property is left as a TODO.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
