| Message ID | 1398877623-16930-5-git-send-email-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 30, 2014 at 10:07:03AM -0700, Matt Roper wrote: > Intel hardware allows the primary plane to be disabled independently of > the CRTC. Provide custom primary plane handling to allow this. > > 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.roper@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 194 ++++++++++++++++++++++++++++++++++- > 1 file changed, 191 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 04bd821..33ed52d 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -39,8 +39,35 @@ > #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_gen3[] = { nit: I'd call it _gen2 since we usually name things based on the oldest thing that supports it. > + 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, > +}; > + > static void intel_increase_pllclock(struct drm_crtc *crtc); > static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on); > > @@ -1909,7 +1936,8 @@ static void intel_disable_primary_hw_plane(struct drm_i915_private *dev_priv, > int reg; > u32 val; > > - WARN(!intel_crtc->primary_enabled, "Primary plane already disabled\n"); > + if (!intel_crtc->primary_enabled) > + return; > > intel_crtc->primary_enabled = false; > > @@ -10551,17 +10579,177 @@ 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 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: > + /* > + * N.B. The DRM setplane code will update the plane->fb pointer after > + * we finish here. > + */ > + intel_unpin_fb_obj(to_intel_framebuffer(plane->fb)->obj); > + > + 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 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->config.pipe_src_w, > + .y2 = intel_crtc->config.pipe_src_h, These might contain garbage if the pipe isn't enabled. The sprite code initializes the clip size to 0 in that case, which neatly results in visible==false always. > + }; > + 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 (!visible) > + return intel_primary_plane_disable(plane); Here we unpin the old fb... > + > + /* > + * 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) > + /* Pin and return without programming hardware */ > + return intel_pin_and_fence_fb_obj(dev, > + to_intel_framebuffer(fb)->obj, > + NULL); ...but here we just pin the new fb and leave the old also pinned? Something's a bit fishy here. Also a non-enabled crtc but visible primary plane doesn't seem to make sense. We also need to remember that set_base will always unpin the old_fb. So if something can result in set_base being called w/ old_fb!=NULL when it's not pinned, we'll be in deep doodoo. > + > + intel_crtc_wait_for_pending_flips(crtc); > + 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_gen3; > + num_formats = ARRAY_SIZE(intel_primary_formats_gen3); > + } else { > + intel_primary_formats = intel_primary_formats_gen4; > + num_formats = ARRAY_SIZE(intel_primary_formats_gen4); > + } > + > + drm_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
On Thu, May 15, 2014 at 06:52:28PM +0300, Ville Syrjälä wrote: ... > > > + }; > > + 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 (!visible) > > + return intel_primary_plane_disable(plane); > > Here we unpin the old fb... > > > + > > + /* > > + * 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) > > + /* Pin and return without programming hardware */ > > + return intel_pin_and_fence_fb_obj(dev, > > + to_intel_framebuffer(fb)->obj, > > + NULL); > > ...but here we just pin the new fb and leave the old also pinned? > > Something's a bit fishy here. Also a non-enabled crtc but visible primary > plane doesn't seem to make sense. We also need to remember that set_base > will always unpin the old_fb. So if something can result in set_base > being called w/ old_fb!=NULL when it's not pinned, we'll be in deep > doodoo. Right, I guess we need to unpin the old fb, if any, here as well in case they perform several setplane() calls while the crtc is disabled. Eventually the crtc will get reenabled by a drmModeSetCrtc call. If we do setcrtc(fb = -1), then it should keep whatever fb we've already pinned via the setplane. If they provide a new fb, then the pinning we're doing here will get unpinned by the set_base that gets called. I don't see a way that you can hit set_base with an unpinned old_fb!=NULL (since the disable plane case farther up also clears primary->fb). Matt > > > + > > + intel_crtc_wait_for_pending_flips(crtc); > > + 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_gen3; > > + num_formats = ARRAY_SIZE(intel_primary_formats_gen3); > > + } else { > > + intel_primary_formats = intel_primary_formats_gen4; > > + num_formats = ARRAY_SIZE(intel_primary_formats_gen4); > > + } > > + > > + drm_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 > > -- > Ville Syrjälä > Intel OTC
On Thu, May 15, 2014 at 09:37:55AM -0700, Matt Roper wrote: > On Thu, May 15, 2014 at 06:52:28PM +0300, Ville Syrjälä wrote: > ... > > > > > + }; > > > + 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 (!visible) > > > + return intel_primary_plane_disable(plane); > > > > Here we unpin the old fb... > > > > > + > > > + /* > > > + * 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) > > > + /* Pin and return without programming hardware */ > > > + return intel_pin_and_fence_fb_obj(dev, > > > + to_intel_framebuffer(fb)->obj, > > > + NULL); > > > > ...but here we just pin the new fb and leave the old also pinned? > > > > Something's a bit fishy here. Also a non-enabled crtc but visible primary > > plane doesn't seem to make sense. We also need to remember that set_base > > will always unpin the old_fb. So if something can result in set_base > > being called w/ old_fb!=NULL when it's not pinned, we'll be in deep > > doodoo. > > Right, I guess we need to unpin the old fb, if any, here as well in case > they perform several setplane() calls while the crtc is disabled. > > Eventually the crtc will get reenabled by a drmModeSetCrtc call. If we > do setcrtc(fb = -1), then it should keep whatever fb we've already > pinned via the setplane. If they provide a new fb, then the pinning > we're doing here will get unpinned by the set_base that gets called. > > I don't see a way that you can hit set_base with an unpinned > old_fb!=NULL (since the disable plane case farther up also clears > primary->fb). But it doesn't. > > > Matt > > > > > > + > > > + intel_crtc_wait_for_pending_flips(crtc); > > > + 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_gen3; > > > + num_formats = ARRAY_SIZE(intel_primary_formats_gen3); > > > + } else { > > > + intel_primary_formats = intel_primary_formats_gen4; > > > + num_formats = ARRAY_SIZE(intel_primary_formats_gen4); > > > + } > > > + > > > + drm_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 > > > > -- > > Ville Syrjälä > > Intel OTC > > -- > Matt Roper > Graphics Software Engineer > IoTG Platform Enabling & Development > Intel Corporation > (916) 356-2795
On Thu, May 15, 2014 at 08:00:48PM +0300, Ville Syrjälä wrote: > On Thu, May 15, 2014 at 09:37:55AM -0700, Matt Roper wrote: > > On Thu, May 15, 2014 at 06:52:28PM +0300, Ville Syrjälä wrote: > > ... > > > > > > > + }; > > > > + 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 (!visible) > > > > + return intel_primary_plane_disable(plane); > > > > > > Here we unpin the old fb... > > > > > > > + > > > > + /* > > > > + * 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) > > > > + /* Pin and return without programming hardware */ > > > > + return intel_pin_and_fence_fb_obj(dev, > > > > + to_intel_framebuffer(fb)->obj, > > > > + NULL); > > > > > > ...but here we just pin the new fb and leave the old also pinned? > > > > > > Something's a bit fishy here. Also a non-enabled crtc but visible primary > > > plane doesn't seem to make sense. We also need to remember that set_base > > > will always unpin the old_fb. So if something can result in set_base > > > being called w/ old_fb!=NULL when it's not pinned, we'll be in deep > > > doodoo. > > > > Right, I guess we need to unpin the old fb, if any, here as well in case > > they perform several setplane() calls while the crtc is disabled. > > > > Eventually the crtc will get reenabled by a drmModeSetCrtc call. If we > > do setcrtc(fb = -1), then it should keep whatever fb we've already > > pinned via the setplane. If they provide a new fb, then the pinning > > we're doing here will get unpinned by the set_base that gets called. > > > > I don't see a way that you can hit set_base with an unpinned > > old_fb!=NULL (since the disable plane case farther up also clears > > primary->fb). > > But it doesn't. > Ah, you're right. I was conflating explicit disables (fb=0) with implicit disables (clipped to invisible). I think the v7 I just sent should handle this properly...for the implicit disable case we leave the fb pinned and pointed to by primary->fb. So when we switch to another fb (or explicitly disable with fb=0), we should unpin it properly. Matt > > > > > > > Matt > > > > > > > > > + > > > > + intel_crtc_wait_for_pending_flips(crtc); > > > > + 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_gen3; > > > > + num_formats = ARRAY_SIZE(intel_primary_formats_gen3); > > > > + } else { > > > > + intel_primary_formats = intel_primary_formats_gen4; > > > > + num_formats = ARRAY_SIZE(intel_primary_formats_gen4); > > > > + } > > > > + > > > > + drm_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 > > > > > > -- > > > Ville Syrjälä > > > Intel OTC > > > > -- > > Matt Roper > > Graphics Software Engineer > > IoTG Platform Enabling & Development > > Intel Corporation > > (916) 356-2795 > > -- > Ville Syrjälä > Intel OTC
On Thu, May 15, 2014 at 12:35:17PM -0700, Matt Roper wrote: > On Thu, May 15, 2014 at 08:00:48PM +0300, Ville Syrjälä wrote: > > On Thu, May 15, 2014 at 09:37:55AM -0700, Matt Roper wrote: > > > On Thu, May 15, 2014 at 06:52:28PM +0300, Ville Syrjälä wrote: > > > ... > > > > > > > > > + }; > > > > > + 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 (!visible) > > > > > + return intel_primary_plane_disable(plane); > > > > > > > > Here we unpin the old fb... > > > > > > > > > + > > > > > + /* > > > > > + * 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) > > > > > + /* Pin and return without programming hardware */ > > > > > + return intel_pin_and_fence_fb_obj(dev, > > > > > + to_intel_framebuffer(fb)->obj, > > > > > + NULL); > > > > > > > > ...but here we just pin the new fb and leave the old also pinned? > > > > > > > > Something's a bit fishy here. Also a non-enabled crtc but visible primary > > > > plane doesn't seem to make sense. We also need to remember that set_base > > > > will always unpin the old_fb. So if something can result in set_base > > > > being called w/ old_fb!=NULL when it's not pinned, we'll be in deep > > > > doodoo. > > > > > > Right, I guess we need to unpin the old fb, if any, here as well in case > > > they perform several setplane() calls while the crtc is disabled. > > > > > > Eventually the crtc will get reenabled by a drmModeSetCrtc call. If we > > > do setcrtc(fb = -1), then it should keep whatever fb we've already > > > pinned via the setplane. If they provide a new fb, then the pinning > > > we're doing here will get unpinned by the set_base that gets called. > > > > > > I don't see a way that you can hit set_base with an unpinned > > > old_fb!=NULL (since the disable plane case farther up also clears > > > primary->fb). > > > > But it doesn't. > > > > Ah, you're right. I was conflating explicit disables (fb=0) with > implicit disables (clipped to invisible). I think the v7 I just sent > should handle this properly...for the implicit disable case we leave the > fb pinned and pointed to by primary->fb. So when we switch to another > fb (or explicitly disable with fb=0), we should unpin it properly. Do we have proper coverage for this fun in our primary plane helper tests? This is the kind of complexity that freaks me out ;-) -Daniel
On Thu, May 15, 2014 at 10:49:52PM +0200, Daniel Vetter wrote: > On Thu, May 15, 2014 at 12:35:17PM -0700, Matt Roper wrote: > > On Thu, May 15, 2014 at 08:00:48PM +0300, Ville Syrjälä wrote: > > > On Thu, May 15, 2014 at 09:37:55AM -0700, Matt Roper wrote: > > > > On Thu, May 15, 2014 at 06:52:28PM +0300, Ville Syrjälä wrote: > > > > ... > > > > > > > > > > > + }; > > > > > > + 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 (!visible) > > > > > > + return intel_primary_plane_disable(plane); > > > > > > > > > > Here we unpin the old fb... > > > > > > > > > > > + > > > > > > + /* > > > > > > + * 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) > > > > > > + /* Pin and return without programming hardware */ > > > > > > + return intel_pin_and_fence_fb_obj(dev, > > > > > > + to_intel_framebuffer(fb)->obj, > > > > > > + NULL); > > > > > > > > > > ...but here we just pin the new fb and leave the old also pinned? > > > > > > > > > > Something's a bit fishy here. Also a non-enabled crtc but visible primary > > > > > plane doesn't seem to make sense. We also need to remember that set_base > > > > > will always unpin the old_fb. So if something can result in set_base > > > > > being called w/ old_fb!=NULL when it's not pinned, we'll be in deep > > > > > doodoo. > > > > > > > > Right, I guess we need to unpin the old fb, if any, here as well in case > > > > they perform several setplane() calls while the crtc is disabled. > > > > > > > > Eventually the crtc will get reenabled by a drmModeSetCrtc call. If we > > > > do setcrtc(fb = -1), then it should keep whatever fb we've already > > > > pinned via the setplane. If they provide a new fb, then the pinning > > > > we're doing here will get unpinned by the set_base that gets called. > > > > > > > > I don't see a way that you can hit set_base with an unpinned > > > > old_fb!=NULL (since the disable plane case farther up also clears > > > > primary->fb). > > > > > > But it doesn't. > > > > > > > Ah, you're right. I was conflating explicit disables (fb=0) with > > implicit disables (clipped to invisible). I think the v7 I just sent > > should handle this properly...for the implicit disable case we leave the > > fb pinned and pointed to by primary->fb. So when we switch to another > > fb (or explicitly disable with fb=0), we should unpin it properly. > > Do we have proper coverage for this fun in our primary plane helper tests? > This is the kind of complexity that freaks me out ;-) > -Daniel Was 'helper' in your question above a typo? The i-g-t tests I've written have been intended for use with this patch (i.e., i915-specific primary plane support), so I don't really have any tests that only test the lesser, helper-provided functionality (and drivers using the helpers shouldn't run into the things Ville is pointing out here because they can't disable the primary plane independent of the crtc). But assuming you meant the general i-g-t tests, yeah, I also posted a slightly updated version so that it now tries to set multiple fb's while the crtc is off. Since crtc=off causes the primary plane to be fully clipped and implicitly disabled, it should exercise these cases and catch the pin/unpin mistakes that Ville's review caught. Matt
On Thu, May 15, 2014 at 01:59:39PM -0700, Matt Roper wrote: > On Thu, May 15, 2014 at 10:49:52PM +0200, Daniel Vetter wrote: > > On Thu, May 15, 2014 at 12:35:17PM -0700, Matt Roper wrote: > > > On Thu, May 15, 2014 at 08:00:48PM +0300, Ville Syrjälä wrote: > > > > On Thu, May 15, 2014 at 09:37:55AM -0700, Matt Roper wrote: > > > > > On Thu, May 15, 2014 at 06:52:28PM +0300, Ville Syrjälä wrote: > > > > > ... > > > > > > > > > > > > > + }; > > > > > > > + 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 (!visible) > > > > > > > + return intel_primary_plane_disable(plane); > > > > > > > > > > > > Here we unpin the old fb... > > > > > > > > > > > > > + > > > > > > > + /* > > > > > > > + * 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) > > > > > > > + /* Pin and return without programming hardware */ > > > > > > > + return intel_pin_and_fence_fb_obj(dev, > > > > > > > + to_intel_framebuffer(fb)->obj, > > > > > > > + NULL); > > > > > > > > > > > > ...but here we just pin the new fb and leave the old also pinned? > > > > > > > > > > > > Something's a bit fishy here. Also a non-enabled crtc but visible primary > > > > > > plane doesn't seem to make sense. We also need to remember that set_base > > > > > > will always unpin the old_fb. So if something can result in set_base > > > > > > being called w/ old_fb!=NULL when it's not pinned, we'll be in deep > > > > > > doodoo. > > > > > > > > > > Right, I guess we need to unpin the old fb, if any, here as well in case > > > > > they perform several setplane() calls while the crtc is disabled. > > > > > > > > > > Eventually the crtc will get reenabled by a drmModeSetCrtc call. If we > > > > > do setcrtc(fb = -1), then it should keep whatever fb we've already > > > > > pinned via the setplane. If they provide a new fb, then the pinning > > > > > we're doing here will get unpinned by the set_base that gets called. > > > > > > > > > > I don't see a way that you can hit set_base with an unpinned > > > > > old_fb!=NULL (since the disable plane case farther up also clears > > > > > primary->fb). > > > > > > > > But it doesn't. > > > > > > > > > > Ah, you're right. I was conflating explicit disables (fb=0) with > > > implicit disables (clipped to invisible). I think the v7 I just sent > > > should handle this properly...for the implicit disable case we leave the > > > fb pinned and pointed to by primary->fb. So when we switch to another > > > fb (or explicitly disable with fb=0), we should unpin it properly. > > > > Do we have proper coverage for this fun in our primary plane helper tests? > > This is the kind of complexity that freaks me out ;-) > > -Daniel > > Was 'helper' in your question above a typo? The i-g-t tests I've > written have been intended for use with this patch (i.e., i915-specific > primary plane support), so I don't really have any tests that only test > the lesser, helper-provided functionality (and drivers using the helpers > shouldn't run into the things Ville is pointing out here because they > can't disable the primary plane independent of the crtc). > > But assuming you meant the general i-g-t tests, yeah, I also posted a > slightly updated version so that it now tries to set multiple fb's while > the crtc is off. Since crtc=off causes the primary plane to be fully > clipped and implicitly disabled, it should exercise these cases and > catch the pin/unpin mistakes that Ville's review caught. Yeah, s/helper// ;-) For the tests, do you also have some that use the implicit fb (i.e. fb = -1) with setcrtc? Just kinda for completeness. -Daniel
On Thu, May 15, 2014 at 11:20:39PM +0200, Daniel Vetter wrote: > > > > Ah, you're right. I was conflating explicit disables (fb=0) with > > > > implicit disables (clipped to invisible). I think the v7 I just sent > > > > should handle this properly...for the implicit disable case we leave the > > > > fb pinned and pointed to by primary->fb. So when we switch to another > > > > fb (or explicitly disable with fb=0), we should unpin it properly. > > > > > > Do we have proper coverage for this fun in our primary plane helper tests? > > > This is the kind of complexity that freaks me out ;-) > > > -Daniel > > > > Was 'helper' in your question above a typo? The i-g-t tests I've > > written have been intended for use with this patch (i.e., i915-specific > > primary plane support), so I don't really have any tests that only test > > the lesser, helper-provided functionality (and drivers using the helpers > > shouldn't run into the things Ville is pointing out here because they > > can't disable the primary plane independent of the crtc). > > > > But assuming you meant the general i-g-t tests, yeah, I also posted a > > slightly updated version so that it now tries to set multiple fb's while > > the crtc is off. Since crtc=off causes the primary plane to be fully > > clipped and implicitly disabled, it should exercise these cases and > > catch the pin/unpin mistakes that Ville's review caught. > > Yeah, s/helper// ;-) For the tests, do you also have some that use the > implicit fb (i.e. fb = -1) with setcrtc? Just kinda for completeness. > -Daniel Yeah...turn off the crtc, set a few different fb's via setplane, then turn the crtc back on with drmModeSetCrtc(fb = -1) and test that it pops up with the right framebuffer set. Matt
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 04bd821..33ed52d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -39,8 +39,35 @@ #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_gen3[] = { + 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, +}; + static void intel_increase_pllclock(struct drm_crtc *crtc); static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on); @@ -1909,7 +1936,8 @@ static void intel_disable_primary_hw_plane(struct drm_i915_private *dev_priv, int reg; u32 val; - WARN(!intel_crtc->primary_enabled, "Primary plane already disabled\n"); + if (!intel_crtc->primary_enabled) + return; intel_crtc->primary_enabled = false; @@ -10551,17 +10579,177 @@ 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 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: + /* + * N.B. The DRM setplane code will update the plane->fb pointer after + * we finish here. + */ + intel_unpin_fb_obj(to_intel_framebuffer(plane->fb)->obj); + + 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 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->config.pipe_src_w, + .y2 = intel_crtc->config.pipe_src_h, + }; + 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 (!visible) + return intel_primary_plane_disable(plane); + + /* + * 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) + /* 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); + 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_gen3; + num_formats = ARRAY_SIZE(intel_primary_formats_gen3); + } else { + intel_primary_formats = intel_primary_formats_gen4; + num_formats = ARRAY_SIZE(intel_primary_formats_gen4); + } + + drm_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++) {
Intel hardware allows the primary plane to be disabled independently of the CRTC. Provide custom primary plane handling to allow this. 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.roper@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 194 ++++++++++++++++++++++++++++++++++- 1 file changed, 191 insertions(+), 3 deletions(-)