Message ID | 20171011160455.1874-7-ville.syrjala@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Wed, Oct 11, 2017 at 07:04:52PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > The only relevant difference between i9xx_get_initial_plane_config() and > ironlake_get_initial_plane_config() is the HSW/BDW TILEOFF handling. > Add that to i9xx_get_initial_plane_config() and nuke > ironlake_get_initial_plane_config(). > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> I'm still kinda wanting some way to test this here with every modeset, but still can't come up with something simple&neat. > --- > drivers/gpu/drm/i915/intel_display.c | 79 +++--------------------------------- > 1 file changed, 6 insertions(+), 73 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 21160a06ab36..82be2342d1c6 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -7490,7 +7490,10 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc, > fourcc = i9xx_format_to_fourcc(pixel_format); > fb->format = drm_format_info(fourcc); > > - if (INTEL_GEN(dev_priv) >= 4) { > + if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > + offset = I915_READ(DSPOFFSET(plane_id)); > + base = I915_READ(DSPSURF(plane_id)) & 0xfffff000; > + } else if (INTEL_GEN(dev_priv) >= 4) { > if (plane_config->tiling) > offset = I915_READ(DSPTILEOFF(plane_id)); > else > @@ -8592,76 +8595,6 @@ static void ironlake_get_pfit_config(struct intel_crtc *crtc, > } > } > > -static void > -ironlake_get_initial_plane_config(struct intel_crtc *crtc, > - struct intel_initial_plane_config *plane_config) > -{ > - struct drm_device *dev = crtc->base.dev; > - struct drm_i915_private *dev_priv = to_i915(dev); > - struct intel_plane *plane = to_intel_plane(crtc->base.primary); > - enum old_plane_id plane_id = plane->plane; > - enum pipe pipe = crtc->pipe; > - u32 val, base, offset; > - int fourcc, pixel_format; > - unsigned int aligned_height; > - struct drm_framebuffer *fb; > - struct intel_framebuffer *intel_fb; > - > - val = I915_READ(DSPCNTR(plane_id)); > - if (!(val & DISPLAY_PLANE_ENABLE)) Hm, should we use plane->get_hw_state to take out this check? At least to partially get close to the hw state readout approach we have for crtc/encoders/connectors. Aside form these two notes, patch itself looks good. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > - return; > - > - intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL); > - if (!intel_fb) { > - DRM_DEBUG_KMS("failed to alloc fb\n"); > - return; > - } > - > - fb = &intel_fb->base; > - > - fb->dev = dev; > - > - if (INTEL_GEN(dev_priv) >= 4) { > - if (val & DISPPLANE_TILED) { > - plane_config->tiling = I915_TILING_X; > - fb->modifier = I915_FORMAT_MOD_X_TILED; > - } > - } > - > - pixel_format = val & DISPPLANE_PIXFORMAT_MASK; > - fourcc = i9xx_format_to_fourcc(pixel_format); > - fb->format = drm_format_info(fourcc); > - > - base = I915_READ(DSPSURF(plane_id)) & 0xfffff000; > - if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > - offset = I915_READ(DSPOFFSET(plane_id)); > - } else { > - if (plane_config->tiling) > - offset = I915_READ(DSPTILEOFF(plane_id)); > - else > - offset = I915_READ(DSPLINOFF(plane_id)); > - } > - plane_config->base = base; > - > - val = I915_READ(PIPESRC(pipe)); > - fb->width = ((val >> 16) & 0xfff) + 1; > - fb->height = ((val >> 0) & 0xfff) + 1; > - > - val = I915_READ(DSPSTRIDE(plane_id)); > - fb->pitches[0] = val & 0xffffffc0; > - > - aligned_height = intel_fb_align_height(fb, 0, fb->height); > - > - plane_config->size = fb->pitches[0] * aligned_height; > - > - DRM_DEBUG_KMS("%s/%s with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n", > - crtc->base.name, plane->base.name, fb->width, fb->height, > - fb->format->cpp[0] * 8, base, fb->pitches[0], > - plane_config->size); > - > - plane_config->fb = intel_fb; > -} > - > static bool ironlake_get_pipe_config(struct intel_crtc *crtc, > struct intel_crtc_state *pipe_config) > { > @@ -14140,7 +14073,7 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv) > } else if (HAS_DDI(dev_priv)) { > dev_priv->display.get_pipe_config = haswell_get_pipe_config; > dev_priv->display.get_initial_plane_config = > - ironlake_get_initial_plane_config; > + i9xx_get_initial_plane_config; > dev_priv->display.crtc_compute_clock = > haswell_crtc_compute_clock; > dev_priv->display.crtc_enable = haswell_crtc_enable; > @@ -14148,7 +14081,7 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv) > } else if (HAS_PCH_SPLIT(dev_priv)) { > dev_priv->display.get_pipe_config = ironlake_get_pipe_config; > dev_priv->display.get_initial_plane_config = > - ironlake_get_initial_plane_config; > + i9xx_get_initial_plane_config; > dev_priv->display.crtc_compute_clock = > ironlake_crtc_compute_clock; > dev_priv->display.crtc_enable = ironlake_crtc_enable; > -- > 2.13.6 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Oct 12, 2017 at 09:17:40PM +0200, Daniel Vetter wrote: > On Wed, Oct 11, 2017 at 07:04:52PM +0300, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > The only relevant difference between i9xx_get_initial_plane_config() and > > ironlake_get_initial_plane_config() is the HSW/BDW TILEOFF handling. > > Add that to i9xx_get_initial_plane_config() and nuke > > ironlake_get_initial_plane_config(). > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > I'm still kinda wanting some way to test this here with every modeset, but > still can't come up with something simple&neat. > > > --- > > drivers/gpu/drm/i915/intel_display.c | 79 +++--------------------------------- > > 1 file changed, 6 insertions(+), 73 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 21160a06ab36..82be2342d1c6 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -7490,7 +7490,10 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc, > > fourcc = i9xx_format_to_fourcc(pixel_format); > > fb->format = drm_format_info(fourcc); > > > > - if (INTEL_GEN(dev_priv) >= 4) { > > + if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > > + offset = I915_READ(DSPOFFSET(plane_id)); > > + base = I915_READ(DSPSURF(plane_id)) & 0xfffff000; > > + } else if (INTEL_GEN(dev_priv) >= 4) { > > if (plane_config->tiling) > > offset = I915_READ(DSPTILEOFF(plane_id)); > > else > > @@ -8592,76 +8595,6 @@ static void ironlake_get_pfit_config(struct intel_crtc *crtc, > > } > > } > > > > -static void > > -ironlake_get_initial_plane_config(struct intel_crtc *crtc, > > - struct intel_initial_plane_config *plane_config) > > -{ > > - struct drm_device *dev = crtc->base.dev; > > - struct drm_i915_private *dev_priv = to_i915(dev); > > - struct intel_plane *plane = to_intel_plane(crtc->base.primary); > > - enum old_plane_id plane_id = plane->plane; > > - enum pipe pipe = crtc->pipe; > > - u32 val, base, offset; > > - int fourcc, pixel_format; > > - unsigned int aligned_height; > > - struct drm_framebuffer *fb; > > - struct intel_framebuffer *intel_fb; > > - > > - val = I915_READ(DSPCNTR(plane_id)); > > - if (!(val & DISPLAY_PLANE_ENABLE)) > > Hm, should we use plane->get_hw_state to take out this check? At least to > partially get close to the hw state readout approach we have for > crtc/encoders/connectors. I guess we could. We'll still need to read DSPCNTR anyway, but I don't think reading it twice is a real issue. I'll post a follow up with this. > > Aside form these two notes, patch itself looks good. > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > - return; > > - > > - intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL); > > - if (!intel_fb) { > > - DRM_DEBUG_KMS("failed to alloc fb\n"); > > - return; > > - } > > - > > - fb = &intel_fb->base; > > - > > - fb->dev = dev; > > - > > - if (INTEL_GEN(dev_priv) >= 4) { > > - if (val & DISPPLANE_TILED) { > > - plane_config->tiling = I915_TILING_X; > > - fb->modifier = I915_FORMAT_MOD_X_TILED; > > - } > > - } > > - > > - pixel_format = val & DISPPLANE_PIXFORMAT_MASK; > > - fourcc = i9xx_format_to_fourcc(pixel_format); > > - fb->format = drm_format_info(fourcc); > > - > > - base = I915_READ(DSPSURF(plane_id)) & 0xfffff000; > > - if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > > - offset = I915_READ(DSPOFFSET(plane_id)); > > - } else { > > - if (plane_config->tiling) > > - offset = I915_READ(DSPTILEOFF(plane_id)); > > - else > > - offset = I915_READ(DSPLINOFF(plane_id)); > > - } > > - plane_config->base = base; > > - > > - val = I915_READ(PIPESRC(pipe)); > > - fb->width = ((val >> 16) & 0xfff) + 1; > > - fb->height = ((val >> 0) & 0xfff) + 1; > > - > > - val = I915_READ(DSPSTRIDE(plane_id)); > > - fb->pitches[0] = val & 0xffffffc0; > > - > > - aligned_height = intel_fb_align_height(fb, 0, fb->height); > > - > > - plane_config->size = fb->pitches[0] * aligned_height; > > - > > - DRM_DEBUG_KMS("%s/%s with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n", > > - crtc->base.name, plane->base.name, fb->width, fb->height, > > - fb->format->cpp[0] * 8, base, fb->pitches[0], > > - plane_config->size); > > - > > - plane_config->fb = intel_fb; > > -} > > - > > static bool ironlake_get_pipe_config(struct intel_crtc *crtc, > > struct intel_crtc_state *pipe_config) > > { > > @@ -14140,7 +14073,7 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv) > > } else if (HAS_DDI(dev_priv)) { > > dev_priv->display.get_pipe_config = haswell_get_pipe_config; > > dev_priv->display.get_initial_plane_config = > > - ironlake_get_initial_plane_config; > > + i9xx_get_initial_plane_config; > > dev_priv->display.crtc_compute_clock = > > haswell_crtc_compute_clock; > > dev_priv->display.crtc_enable = haswell_crtc_enable; > > @@ -14148,7 +14081,7 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv) > > } else if (HAS_PCH_SPLIT(dev_priv)) { > > dev_priv->display.get_pipe_config = ironlake_get_pipe_config; > > dev_priv->display.get_initial_plane_config = > > - ironlake_get_initial_plane_config; > > + i9xx_get_initial_plane_config; > > dev_priv->display.crtc_compute_clock = > > ironlake_crtc_compute_clock; > > dev_priv->display.crtc_enable = ironlake_crtc_enable; > > -- > > 2.13.6 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 21160a06ab36..82be2342d1c6 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7490,7 +7490,10 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc, fourcc = i9xx_format_to_fourcc(pixel_format); fb->format = drm_format_info(fourcc); - if (INTEL_GEN(dev_priv) >= 4) { + if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { + offset = I915_READ(DSPOFFSET(plane_id)); + base = I915_READ(DSPSURF(plane_id)) & 0xfffff000; + } else if (INTEL_GEN(dev_priv) >= 4) { if (plane_config->tiling) offset = I915_READ(DSPTILEOFF(plane_id)); else @@ -8592,76 +8595,6 @@ static void ironlake_get_pfit_config(struct intel_crtc *crtc, } } -static void -ironlake_get_initial_plane_config(struct intel_crtc *crtc, - struct intel_initial_plane_config *plane_config) -{ - struct drm_device *dev = crtc->base.dev; - struct drm_i915_private *dev_priv = to_i915(dev); - struct intel_plane *plane = to_intel_plane(crtc->base.primary); - enum old_plane_id plane_id = plane->plane; - enum pipe pipe = crtc->pipe; - u32 val, base, offset; - int fourcc, pixel_format; - unsigned int aligned_height; - struct drm_framebuffer *fb; - struct intel_framebuffer *intel_fb; - - val = I915_READ(DSPCNTR(plane_id)); - if (!(val & DISPLAY_PLANE_ENABLE)) - return; - - intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL); - if (!intel_fb) { - DRM_DEBUG_KMS("failed to alloc fb\n"); - return; - } - - fb = &intel_fb->base; - - fb->dev = dev; - - if (INTEL_GEN(dev_priv) >= 4) { - if (val & DISPPLANE_TILED) { - plane_config->tiling = I915_TILING_X; - fb->modifier = I915_FORMAT_MOD_X_TILED; - } - } - - pixel_format = val & DISPPLANE_PIXFORMAT_MASK; - fourcc = i9xx_format_to_fourcc(pixel_format); - fb->format = drm_format_info(fourcc); - - base = I915_READ(DSPSURF(plane_id)) & 0xfffff000; - if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { - offset = I915_READ(DSPOFFSET(plane_id)); - } else { - if (plane_config->tiling) - offset = I915_READ(DSPTILEOFF(plane_id)); - else - offset = I915_READ(DSPLINOFF(plane_id)); - } - plane_config->base = base; - - val = I915_READ(PIPESRC(pipe)); - fb->width = ((val >> 16) & 0xfff) + 1; - fb->height = ((val >> 0) & 0xfff) + 1; - - val = I915_READ(DSPSTRIDE(plane_id)); - fb->pitches[0] = val & 0xffffffc0; - - aligned_height = intel_fb_align_height(fb, 0, fb->height); - - plane_config->size = fb->pitches[0] * aligned_height; - - DRM_DEBUG_KMS("%s/%s with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n", - crtc->base.name, plane->base.name, fb->width, fb->height, - fb->format->cpp[0] * 8, base, fb->pitches[0], - plane_config->size); - - plane_config->fb = intel_fb; -} - static bool ironlake_get_pipe_config(struct intel_crtc *crtc, struct intel_crtc_state *pipe_config) { @@ -14140,7 +14073,7 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv) } else if (HAS_DDI(dev_priv)) { dev_priv->display.get_pipe_config = haswell_get_pipe_config; dev_priv->display.get_initial_plane_config = - ironlake_get_initial_plane_config; + i9xx_get_initial_plane_config; dev_priv->display.crtc_compute_clock = haswell_crtc_compute_clock; dev_priv->display.crtc_enable = haswell_crtc_enable; @@ -14148,7 +14081,7 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv) } else if (HAS_PCH_SPLIT(dev_priv)) { dev_priv->display.get_pipe_config = ironlake_get_pipe_config; dev_priv->display.get_initial_plane_config = - ironlake_get_initial_plane_config; + i9xx_get_initial_plane_config; dev_priv->display.crtc_compute_clock = ironlake_crtc_compute_clock; dev_priv->display.crtc_enable = ironlake_crtc_enable;