[Intel-gfx] [PATCH 3/5] drm/i915: retrieve current fb config into new plane_config structure at init v4
Daniel Vetter
daniel at ffwll.ch
Tue Nov 26 18:43:53 CET 2013
On Mon, Nov 25, 2013 at 03:51:17PM -0800, Jesse Barnes wrote:
> Read out the current plane configuration at init time into a new
> plane_config structure. This allows us to track any existing
> framebuffers attached to the plane and potentially re-use them in our
> fbdev code for a smooth handoff.
>
> v2: update for new pitch_for_width function (Jesse)
> comment how get_plane_config works with shared fbs (Jesse)
> v3: s/ARGB/XRGB (Ville)
> use pipesrc width/height (Ville)
> fix fourcc comment (Bob)
> use drm_format_plane_cpp (Ville)
> v4: use fb for tracking fb data object (Ville)
>
> Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +
> drivers/gpu/drm/i915/intel_display.c | 127 ++++++++++++++++++++++++++++++++++-
> drivers/gpu/drm/i915/intel_drv.h | 8 +++
> 3 files changed, 136 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 14f250a..6569e93 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -365,6 +365,7 @@ struct drm_i915_error_state {
>
> struct intel_connector;
> struct intel_crtc_config;
> +struct intel_plane_config;
> struct intel_crtc;
> struct intel_limit;
> struct dpll;
> @@ -403,6 +404,8 @@ struct drm_i915_display_funcs {
> * fills out the pipe-config with the hw state. */
> bool (*get_pipe_config)(struct intel_crtc *,
> struct intel_crtc_config *);
> + void (*get_plane_config)(struct intel_crtc *,
> + struct intel_plane_config *);
> int (*crtc_mode_set)(struct drm_crtc *crtc,
> int x, int y,
> struct drm_framebuffer *old_fb);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 321d751..089128b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2002,6 +2002,27 @@ unsigned long intel_gen4_compute_page_offset(int *x, int *y,
> }
> }
>
> +int intel_format_to_fourcc(int format)
> +{
> + switch (format) {
> + case DISPPLANE_8BPP:
> + return DRM_FORMAT_C8;
> + case DISPPLANE_BGRX555:
> + return DRM_FORMAT_XRGB1555;
> + case DISPPLANE_BGRX565:
> + return DRM_FORMAT_RGB565;
> + default:
> + case DISPPLANE_BGRX888:
> + return DRM_FORMAT_XRGB8888;
> + case DISPPLANE_RGBX888:
> + return DRM_FORMAT_XBGR8888;
> + case DISPPLANE_BGRX101010:
> + return DRM_FORMAT_XRGB2101010;
> + case DISPPLANE_RGBX101010:
> + return DRM_FORMAT_XBGR2101010;
> + }
> +}
> +
> static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
> int x, int y)
> {
> @@ -5474,6 +5495,95 @@ intel_framebuffer_pitch_for_width(struct drm_i915_private *dev_priv, int width,
> return ALIGN(pitch, align);
> }
>
> +static void i9xx_get_plane_config(struct intel_crtc *crtc,
> + struct intel_plane_config *plane_config)
> +{
> + struct drm_device *dev = crtc->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_i915_gem_object *obj = NULL;
> + struct drm_mode_fb_cmd2 mode_cmd = { 0 };
> + u32 val, base, offset;
> + int pipe = crtc->pipe, plane = crtc->plane;
> + int fourcc, pixel_format;
> +
> + plane_config->fb = kzalloc(sizeof(*plane_config->fb), GFP_KERNEL);
> + if (!plane_config->fb) {
> + DRM_DEBUG_KMS("failed to alloc fb\n");
> + return;
> + }
> +
> + val = I915_READ(DSPCNTR(plane));
> +
> + if (INTEL_INFO(dev)->gen >= 4)
> + if (val & DISPPLANE_TILED)
> + plane_config->tiled = true;
> +
> + pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
> + fourcc = intel_format_to_fourcc(pixel_format);
> + plane_config->fb->base.pixel_format = fourcc;
> + plane_config->fb->base.bits_per_pixel =
> + drm_format_plane_cpp(fourcc, 0) * 8;
> +
> + if (INTEL_INFO(dev)->gen >= 4) {
> + if (plane_config->tiled)
> + offset = I915_READ(DSPTILEOFF(plane));
> + else
> + offset = I915_READ(DSPLINOFF(plane));
> + base = I915_READ(DSPSURF(plane)) & 0xfffff000;
> + } else {
> + base = I915_READ(DSPADDR(plane));
> + }
> +
> + val = I915_READ(PIPESRC(pipe));
> + plane_config->fb->base.width = ((val >> 16) & 0xfff) + 1;
> + plane_config->fb->base.height = ((val >> 0) & 0xfff) + 1;
> +
> + plane_config->fb->base.pitches[0] =
> + intel_framebuffer_pitch_for_width(dev_priv,
> + plane_config->fb->base.width,
> + plane_config->fb->base.bits_per_pixel,
> + plane_config->tiled);
Shouldn't we read out the stride from the respective hw register, too?
> +
> + plane_config->size = ALIGN(plane_config->fb->base.pitches[0] *
> + plane_config->fb->base.height, PAGE_SIZE);
If you bother with tiling I think you also need to bother with correct
size alignement.
> +
> + DRM_DEBUG_KMS("pipe/plane %d/%d with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
> + pipe, plane, plane_config->fb->base.width,
> + plane_config->fb->base.height,
> + plane_config->fb->base.bits_per_pixel, base,
> + plane_config->fb->base.pitches[0],
> + plane_config->size);
> +
> + /*
> + * If the fb is shared between multiple heads, we'll just get the
> + * first one.
> + */
> + obj = i915_gem_object_create_stolen_for_preallocated(dev, base, base,
> + plane_config->size);
> + if (!obj)
> + return;
> +
> + mode_cmd.pixel_format = fourcc;
> + mode_cmd.width = plane_config->fb->base.width;
> + mode_cmd.height = plane_config->fb->base.height;
> + mode_cmd.pitches[0] = plane_config->fb->base.pitches[0];
> +
> + mutex_lock(&dev->struct_mutex);
> +
> + if (intel_framebuffer_init(dev, plane_config->fb, &mode_cmd, obj)) {
> + DRM_DEBUG_KMS("intel fb init failed\n");
> + goto out_unref_obj;
> + }
> +
> + mutex_unlock(&dev->struct_mutex);
> + DRM_DEBUG_KMS("plane fb obj %p\n", plane_config->fb->obj);
> + return;
> +
> +out_unref_obj:
> + drm_gem_object_unreference(&obj->base);
> + mutex_unlock(&dev->struct_mutex);
> +}
> +
> static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> struct intel_crtc_config *pipe_config)
> {
> @@ -10562,6 +10672,7 @@ static void intel_init_display(struct drm_device *dev)
> dev_priv->display.update_plane = ironlake_update_plane;
Sad Haswell is sad it seems.
> } else if (IS_VALLEYVIEW(dev)) {
> dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
> + dev_priv->display.get_plane_config = i9xx_get_plane_config;
> dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
> dev_priv->display.crtc_enable = valleyview_crtc_enable;
> dev_priv->display.crtc_disable = i9xx_crtc_disable;
> @@ -10569,6 +10680,7 @@ static void intel_init_display(struct drm_device *dev)
> dev_priv->display.update_plane = i9xx_update_plane;
> } else {
> dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
> + dev_priv->display.get_plane_config = i9xx_get_plane_config;
> dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
> dev_priv->display.crtc_enable = i9xx_crtc_enable;
> dev_priv->display.crtc_disable = i9xx_crtc_disable;
> @@ -10823,6 +10935,7 @@ void intel_modeset_suspend_hw(struct drm_device *dev)
> void intel_modeset_init(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_crtc *crtc;
> int i, j, ret;
>
> drm_mode_config_init(dev);
> @@ -10879,6 +10992,18 @@ void intel_modeset_init(struct drm_device *dev)
>
> /* Just in case the BIOS is doing something questionable. */
> intel_disable_fbc(dev);
> +
> + intel_modeset_setup_hw_state(dev, false);
> +
> + list_for_each_entry(crtc, &dev->mode_config.crtc_list,
> + base.head) {
> + if (!crtc->active)
> + continue;
> +
> + if (dev_priv->display.get_plane_config)
> + dev_priv->display.get_plane_config(crtc,
> + &crtc->plane_config);
> + }
If we go with Chris' suggestion to disable the crtc if we can't get at a
sane framebuffer for it then this would make much more sense in the
hardware state readout code. Especially since always having framebuffers
around would allow us to ditch a bit of special-casing code all over the
place.
> }
>
> static void
> @@ -11248,8 +11373,6 @@ void intel_modeset_gem_init(struct drm_device *dev)
> intel_modeset_init_hw(dev);
>
> intel_setup_overlay(dev);
> -
> - intel_modeset_setup_hw_state(dev, false);
> }
>
> void intel_modeset_cleanup(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 0231281..59f8f38 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -209,6 +209,12 @@ typedef struct dpll {
> int p;
> } intel_clock_t;
>
> +struct intel_plane_config {
> + struct intel_framebuffer *fb; /* ends up managed by intel_fbdev.c */
> + bool tiled;
> + int size;
> +};
> +
> struct intel_crtc_config {
> /**
> * quirks - bitfield with hw state readout quirks
> @@ -358,6 +364,7 @@ struct intel_crtc {
> bool cursor_visible;
>
> struct intel_crtc_config config;
> + struct intel_plane_config plane_config;
>
> uint32_t ddi_pll_sel;
>
> @@ -707,6 +714,7 @@ void hsw_enable_ips(struct intel_crtc *crtc);
> void hsw_disable_ips(struct intel_crtc *crtc);
> void intel_display_set_init_power(struct drm_device *dev, bool enable);
> int valleyview_get_vco(struct drm_i915_private *dev_priv);
> +int intel_format_to_fourcc(int format);
>
> /* intel_dp.c */
> void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
> --
> 1.8.4.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the Intel-gfx
mailing list