On 05/15 12:21, Matt Roper wrote:
> Intel hardware allows the primary plane to be disabled independently of
> the CRTC.  Provide custom primary plane handling to allow this.
> 
> v7:
>  - Clip primary plane to invisible when crtc is disabled since
>    intel_crtc->config.pipe_src_{w,h} may be garbage otherwise.
>  - Unpin old fb before pinning new one in the "just pin and
>    return" case that is used when the crtc is disabled.
>  - Don't treat implicit disabling of the primary plane (caused by
>    clipping) the same way as explicit disabling (caused by fb=0).
>    For implicit disables, we should leave the fb set and pinned,
>    whereas for explicit disables we need to unpin the fb before
>    primary->fb is cleared.
> v6:
>  - Pass rectangles to primary helper check function and get plane
>    visibility back.
>  - Wait for pending pageflips on primary plane update/disable.
>  - Allow primary plane to be updated while the crtc is disabled (changes
>    will take effect when the crtc is re-enabled if modeset passes -1
>    for the fb id).
>  - Drop WARN() if we try to disable the primary plane when it's
>    already been disabled.  This will happen if the crtc gets disabled
>    after the primary plane has already been disabled independently.
> v5:
>  - Use new drm_primary_helper_check_update() helper function to check
>    setplane parameter validity.
>  - Swap primary plane's pipe for pre-gen4 FBC (caught by Ville Syrjälä)
>  - Cleanup primary plane properly on crtc init failure
> v4:
>  - Don't add a primary_plane field to intel_crtc; that was left over
>    from a much earlier iteration of this patch series, but is no longer
>    needed/used now that the DRM core primary plane support has been
>    merged.
> v3:
>  - Provide gen-specific primary plane format lists (suggested by Daniel
>    Vetter).
>  - If the primary plane is already enabled, go ahead and just call the
>    primary plane helper to do the update (suggested by Daniel Vetter).
>  - Don't try to disable the primary plane on destruction; the DRM layer
>    should have already taken care of this for us.
> v2:
>  - Unpin fb properly on primary plane disable
>  - Provide an Intel-specific set of primary plane formats
>  - Additional sanity checks on setplane (in line with the checks
>    currently being done by the DRM core primary plane helper)
> 
> Signed-off-by: Matt Roper <matthew.d.ro...@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 212 
> ++++++++++++++++++++++++++++++++++-
>  1 file changed, 209 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 3aedc64..e9f196e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -39,10 +39,37 @@
>  #include "i915_trace.h"
>  #include <drm/drm_dp_helper.h>
>  #include <drm/drm_crtc_helper.h>
> +#include <drm/drm_plane_helper.h>
> +#include <drm/drm_rect.h>
>  #include <linux/dma_remapping.h>
>  
> +/* Primary plane formats supported by all gen */
> +#define COMMON_PRIMARY_FORMATS \
> +     DRM_FORMAT_C8, \
> +     DRM_FORMAT_RGB565, \
> +     DRM_FORMAT_XRGB8888, \
> +     DRM_FORMAT_ARGB8888
> +
> +/* Primary plane formats for gen <= 3 */
> +static const uint32_t intel_primary_formats_gen2[] = {
> +     COMMON_PRIMARY_FORMATS,
> +     DRM_FORMAT_XRGB1555,
> +     DRM_FORMAT_ARGB1555,
> +};
> +
> +/* Primary plane formats for gen >= 4 */
> +static const uint32_t intel_primary_formats_gen4[] = {
> +     COMMON_PRIMARY_FORMATS, \
> +     DRM_FORMAT_XBGR8888,
> +     DRM_FORMAT_ABGR8888,
> +     DRM_FORMAT_XRGB2101010,
> +     DRM_FORMAT_ARGB2101010,
> +     DRM_FORMAT_XBGR2101010,
> +     DRM_FORMAT_ABGR2101010,
> +};
> +
>  #define DIV_ROUND_CLOSEST_ULL(ll, d) \
> -     ({ unsigned long long _tmp = (ll)+(d)/2; do_div(_tmp, d); _tmp; })
> +({ unsigned long long _tmp = (ll)+(d)/2; do_div(_tmp, d); _tmp; })
>  
>  static void intel_increase_pllclock(struct drm_crtc *crtc);
>  static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
> @@ -10832,17 +10859,196 @@ static void intel_shared_dpll_init(struct 
> drm_device *dev)
>       BUG_ON(dev_priv->num_shared_dpll > I915_NUM_PLLS);
>  }
>  
> +static int
> +intel_primary_plane_disable(struct drm_plane *plane)
> +{
> +     struct drm_device *dev = plane->dev;
> +     struct drm_i915_private *dev_priv = dev->dev_private;
> +     struct intel_plane *intel_plane = to_intel_plane(plane);
> +     struct intel_crtc *intel_crtc;
> +
> +     if (!plane->fb)
> +             return 0;
> +
> +     BUG_ON(!plane->crtc);
> +
> +     intel_crtc = to_intel_crtc(plane->crtc);
> +
> +     /*
> +      * Even though we checked plane->fb above, it's still possible that
> +      * the primary plane has been implicitly disabled because the crtc
> +      * coordinates given weren't visible, or because we detected
> +      * that it was 100% covered by a sprite plane.  Or, the CRTC may be
> +      * off and we've set a fb, but haven't actually turned on the CRTC yet.
> +      * In either case, we need to unpin the FB and let the fb pointer get
> +      * updated, but otherwise we don't need to touch the hardware.
> +      */
> +     if (!intel_crtc->primary_enabled)
> +             goto disable_unpin;
> +
> +     intel_crtc_wait_for_pending_flips(plane->crtc);
> +     intel_disable_primary_hw_plane(dev_priv, intel_plane->plane,
> +                                    intel_plane->pipe);
> +
> +disable_unpin:
> +     intel_unpin_fb_obj(to_intel_framebuffer(plane->fb)->obj);
> +     plane->fb = NULL;
> +
> +     return 0;
> +}
> +
> +static int
> +intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
> +                          struct drm_framebuffer *fb, int crtc_x, int crtc_y,
> +                          unsigned int crtc_w, unsigned int crtc_h,
> +                          uint32_t src_x, uint32_t src_y,
> +                          uint32_t src_w, uint32_t src_h)
> +{
> +     struct drm_device *dev = crtc->dev;
> +     struct drm_i915_private *dev_priv = dev->dev_private;
> +     struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +     struct intel_plane *intel_plane = to_intel_plane(plane);
> +     struct drm_rect dest = {
> +             /* integer pixels */
> +             .x1 = crtc_x,
> +             .y1 = crtc_y,
> +             .x2 = crtc_x + crtc_w,
> +             .y2 = crtc_y + crtc_h,
> +     };
> +     struct drm_rect src = {
> +             /* 16.16 fixed point */
> +             .x1 = src_x,
> +             .y1 = src_y,
> +             .x2 = src_x + src_w,
> +             .y2 = src_y + src_h,
> +     };
> +     const struct drm_rect clip = {
> +             /* integer pixels */
> +             .x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
> +             .y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
> +     };
> +     bool visible;
> +     int ret;
> +
> +     ret = drm_primary_helper_check_update(plane, crtc, fb,
> +                                           &src, &dest, &clip,
> +                                           DRM_PLANE_HELPER_NO_SCALING,
> +                                           DRM_PLANE_HELPER_NO_SCALING,
> +                                           false, true, &visible);
> +
> +     if (ret)
> +             return ret;
> +
> +     /*
> +      * If the CRTC isn't enabled, we're just pinning the framebuffer,
> +      * updating the fb pointer, and returning without touching the
> +      * hardware.  This allows us to later do a drmModeSetCrtc with fb=-1 to
> +      * turn on the display with all planes setup as desired.
> +      */
> +     if (!crtc->enabled) {
> +             /*
> +              * If we already called setplane while the crtc was disabled,
> +              * we may have an fb pinned; unpin it.
> +              */
> +             if (plane->fb)
> +                     
> intel_unpin_fb_obj(to_intel_framebuffer(plane->fb)->obj);
> +
> +             /* Pin and return without programming hardware */
> +             return intel_pin_and_fence_fb_obj(dev,
> +                                               to_intel_framebuffer(fb)->obj,
> +                                               NULL);
> +     }
> +
> +     intel_crtc_wait_for_pending_flips(crtc);

You may move the intel_crtc_wait_for_pending_flips after below.  If the primary
plane is always cover by another plane, it doesn't need to wait for previously
flip to complete.
> +
> +     /*
> +      * If clipping results in a non-visible primary plane, just disable the
> +      * plane hardware.  Note that this is a bit different than if userspace
> +      * explicitly disables the plane by passing fb=0 because plane->fb
> +      * remains set and pinned until we switch to a different fb.
> +      */
> +     if (!visible && intel_crtc->primary_enabled) {
> +             intel_disable_primary_hw_plane(dev_priv, intel_plane->plane,
> +                                            intel_plane->pipe);
> +             return 0;

If the primary plane is not visible, there is no pin here. In drm_mode_setplane,
plane->fb = fb after this. 

The next call to disable the plane will do unpin the plane->fb which has never
pinned. 

Regards,
Chon Ming

> +     }
> +
> +     ret = intel_pipe_set_base(crtc, src.x1, src.y1, fb);
> +     if (ret)
> +             return ret;
> +
> +     if (!intel_crtc->primary_enabled)
> +             intel_enable_primary_hw_plane(dev_priv, intel_crtc->plane,
> +                                           intel_crtc->pipe);
> +
> +     return 0;
> +}
> +
> +static void intel_primary_plane_destroy(struct drm_plane *plane)
> +{
> +     struct intel_plane *intel_plane = to_intel_plane(plane);
> +     drm_plane_cleanup(plane);
> +     kfree(intel_plane);
> +}
> +
> +static const struct drm_plane_funcs intel_primary_plane_funcs = {
> +     .update_plane = intel_primary_plane_setplane,
> +     .disable_plane = intel_primary_plane_disable,
> +     .destroy = intel_primary_plane_destroy,
> +};
> +
> +static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> +                                                 int pipe)
> +{
> +     struct intel_plane *primary;
> +     const uint32_t *intel_primary_formats;
> +     int num_formats;
> +
> +     primary = kzalloc(sizeof(*primary), GFP_KERNEL);
> +     if (primary == NULL)
> +             return NULL;
> +
> +     primary->can_scale = false;
> +     primary->max_downscale = 1;
> +     primary->pipe = pipe;
> +     primary->plane = pipe;
> +     if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
> +             primary->plane = !pipe;
> +
> +     if (INTEL_INFO(dev)->gen <= 3) {
> +             intel_primary_formats = intel_primary_formats_gen2;
> +             num_formats = ARRAY_SIZE(intel_primary_formats_gen2);
> +     } else {
> +             intel_primary_formats = intel_primary_formats_gen4;
> +             num_formats = ARRAY_SIZE(intel_primary_formats_gen4);
> +     }
> +
> +     drm_universal_plane_init(dev, &primary->base, 0,
> +                              &intel_primary_plane_funcs,
> +                              intel_primary_formats, num_formats,
> +                              DRM_PLANE_TYPE_PRIMARY);
> +     return &primary->base;
> +}
> +
>  static void intel_crtc_init(struct drm_device *dev, int pipe)
>  {
>       struct drm_i915_private *dev_priv = dev->dev_private;
>       struct intel_crtc *intel_crtc;
> -     int i;
> +     struct drm_plane *primary;
> +     int i, ret;
>  
>       intel_crtc = kzalloc(sizeof(*intel_crtc), GFP_KERNEL);
>       if (intel_crtc == NULL)
>               return;
>  
> -     drm_crtc_init(dev, &intel_crtc->base, &intel_crtc_funcs);
> +     primary = intel_primary_plane_create(dev, pipe);
> +     ret = drm_crtc_init_with_planes(dev, &intel_crtc->base, primary,
> +                                     NULL, &intel_crtc_funcs);
> +     if (ret) {
> +             drm_plane_cleanup(primary);
> +             kfree(intel_crtc);
> +             return;
> +     }
>  
>       drm_mode_crtc_set_gamma_size(&intel_crtc->base, 256);
>       for (i = 0; i < 256; i++) {
> -- 
> 1.8.5.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to