diff mbox

[3/8] drm/i915: Unclutter the get_plane() functions

Message ID 1414603367-24964-4-git-send-email-damien.lespiau@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lespiau, Damien Oct. 29, 2014, 5:22 p.m. UTC 
crtc->base.primary->fb was used everywhere. Use fb to temporarily point
there and don't forget to assign fb to its final destination at the end.

v2: Rebase on top of misc changes (mask of DSPSURF, PAGE_ALIGN)

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 61 ++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 34 deletions(-)

Comments

Tvrtko Ursulin Oct. 30, 2014, 5:31 p.m. UTC | #1 
On 10/29/2014 05:22 PM, Damien Lespiau wrote:
> crtc->base.primary->fb was used everywhere. Use fb to temporarily point
> there and don't forget to assign fb to its final destination at the end.
>
> v2: Rebase on top of misc changes (mask of DSPSURF, PAGE_ALIGN)
>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 61 ++++++++++++++++--------------------
>   1 file changed, 27 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2a84e47..6c042eb 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6459,9 +6459,10 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc,
>   	int pipe = crtc->pipe, plane = crtc->plane;
>   	int fourcc, pixel_format;
>   	int aligned_height;
> +	struct drm_framebuffer *fb;
>
> -	crtc->base.primary->fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
> -	if (!crtc->base.primary->fb) {
> +	fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
> +	if (!fb) {
>   		DRM_DEBUG_KMS("failed to alloc fb\n");
>   		return;

Is it of any consequence that after the change crtc->base.primary->fb 
will preserve it's content, while previously it would be NULLed on 
allocation failure? Probably not, just asking...

Tvrtko
Lespiau, Damien Oct. 30, 2014, 5:41 p.m. UTC | #2 
On Thu, Oct 30, 2014 at 05:31:57PM +0000, Tvrtko Ursulin wrote:
> 
> On 10/29/2014 05:22 PM, Damien Lespiau wrote:
> >crtc->base.primary->fb was used everywhere. Use fb to temporarily point
> >there and don't forget to assign fb to its final destination at the end.
> >
> >v2: Rebase on top of misc changes (mask of DSPSURF, PAGE_ALIGN)
> >
> >Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> >---
> >  drivers/gpu/drm/i915/intel_display.c | 61 ++++++++++++++++--------------------
> >  1 file changed, 27 insertions(+), 34 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >index 2a84e47..6c042eb 100644
> >--- a/drivers/gpu/drm/i915/intel_display.c
> >+++ b/drivers/gpu/drm/i915/intel_display.c
> >@@ -6459,9 +6459,10 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc,
> >  	int pipe = crtc->pipe, plane = crtc->plane;
> >  	int fourcc, pixel_format;
> >  	int aligned_height;
> >+	struct drm_framebuffer *fb;
> >
> >-	crtc->base.primary->fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
> >-	if (!crtc->base.primary->fb) {
> >+	fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
> >+	if (!fb) {
> >  		DRM_DEBUG_KMS("failed to alloc fb\n");
> >  		return;
> 
> Is it of any consequence that after the change
> crtc->base.primary->fb will preserve it's content, while previously
> it would be NULLed on allocation failure? Probably not, just
> asking...

The get_plane_config() vfunc is only from intel_modeset_init() (to try
and inherit the BIOS fb) and so at the point, the crtc structure has
just been initialized and the initial get_pipe_config() doesn't touch
this fb field. crtc->base.primary->fb has not other choice than being
NULL at this point then I believe. So shouldn't matter.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2a84e47..6c042eb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6459,9 +6459,10 @@  static void i9xx_get_plane_config(struct intel_crtc *crtc,
 	int pipe = crtc->pipe, plane = crtc->plane;
 	int fourcc, pixel_format;
 	int aligned_height;
+	struct drm_framebuffer *fb;
 
-	crtc->base.primary->fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
-	if (!crtc->base.primary->fb) {
+	fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
+	if (!fb) {
 		DRM_DEBUG_KMS("failed to alloc fb\n");
 		return;
 	}
@@ -6474,9 +6475,8 @@  static void i9xx_get_plane_config(struct intel_crtc *crtc,
 
 	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
 	fourcc = intel_format_to_fourcc(pixel_format);
-	crtc->base.primary->fb->pixel_format = fourcc;
-	crtc->base.primary->fb->bits_per_pixel =
-		drm_format_plane_cpp(fourcc, 0) * 8;
+	fb->pixel_format = fourcc;
+	fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
 
 	if (INTEL_INFO(dev)->gen >= 4) {
 		if (plane_config->tiling)
@@ -6490,26 +6490,22 @@  static void i9xx_get_plane_config(struct intel_crtc *crtc,
 	plane_config->base = base;
 
 	val = I915_READ(PIPESRC(pipe));
-	crtc->base.primary->fb->width = ((val >> 16) & 0xfff) + 1;
-	crtc->base.primary->fb->height = ((val >> 0) & 0xfff) + 1;
+	fb->width = ((val >> 16) & 0xfff) + 1;
+	fb->height = ((val >> 0) & 0xfff) + 1;
 
 	val = I915_READ(DSPSTRIDE(pipe));
-	crtc->base.primary->fb->pitches[0] = val & 0xffffffc0;
+	fb->pitches[0] = val & 0xffffffc0;
 
-	aligned_height = intel_fb_align_height(dev,
-					       crtc->base.primary->fb->height,
+	aligned_height = intel_fb_align_height(dev, fb->height,
 					       plane_config->tiling);
 
-	plane_config->size = PAGE_ALIGN(crtc->base.primary->fb->pitches[0] *
-					aligned_height);
+	plane_config->size = PAGE_ALIGN(fb->pitches[0] * aligned_height);
 
 	DRM_DEBUG_KMS("pipe/plane %d/%d with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
-		      pipe, plane, crtc->base.primary->fb->width,
-		      crtc->base.primary->fb->height,
-		      crtc->base.primary->fb->bits_per_pixel, base,
-		      crtc->base.primary->fb->pitches[0],
-		      plane_config->size);
+		      pipe, plane, fb->width, fb->height, fb->bits_per_pixel,
+		      base, fb->pitches[0], plane_config->size);
 
+	crtc->base.primary->fb = fb;
 }
 
 static void chv_crtc_clock_get(struct intel_crtc *crtc,
@@ -7492,9 +7488,10 @@  static void ironlake_get_plane_config(struct intel_crtc *crtc,
 	int pipe = crtc->pipe, plane = crtc->plane;
 	int fourcc, pixel_format;
 	int aligned_height;
+	struct drm_framebuffer *fb;
 
-	crtc->base.primary->fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
-	if (!crtc->base.primary->fb) {
+	fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
+	if (!fb) {
 		DRM_DEBUG_KMS("failed to alloc fb\n");
 		return;
 	}
@@ -7507,9 +7504,8 @@  static void ironlake_get_plane_config(struct intel_crtc *crtc,
 
 	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
 	fourcc = intel_format_to_fourcc(pixel_format);
-	crtc->base.primary->fb->pixel_format = fourcc;
-	crtc->base.primary->fb->bits_per_pixel =
-		drm_format_plane_cpp(fourcc, 0) * 8;
+	fb->pixel_format = fourcc;
+	fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
 
 	base = I915_READ(DSPSURF(plane)) & 0xfffff000;
 	if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
@@ -7523,25 +7519,22 @@  static void ironlake_get_plane_config(struct intel_crtc *crtc,
 	plane_config->base = base;
 
 	val = I915_READ(PIPESRC(pipe));
-	crtc->base.primary->fb->width = ((val >> 16) & 0xfff) + 1;
-	crtc->base.primary->fb->height = ((val >> 0) & 0xfff) + 1;
+	fb->width = ((val >> 16) & 0xfff) + 1;
+	fb->height = ((val >> 0) & 0xfff) + 1;
 
 	val = I915_READ(DSPSTRIDE(pipe));
-	crtc->base.primary->fb->pitches[0] = val & 0xffffffc0;
+	fb->pitches[0] = val & 0xffffffc0;
 
-	aligned_height = intel_fb_align_height(dev,
-					       crtc->base.primary->fb->height,
+	aligned_height = intel_fb_align_height(dev, fb->height,
 					       plane_config->tiling);
 
-	plane_config->size = PAGE_ALIGN(crtc->base.primary->fb->pitches[0] *
-					aligned_height);
+	plane_config->size = PAGE_ALIGN(fb->pitches[0] * aligned_height);
 
 	DRM_DEBUG_KMS("pipe/plane %d/%d with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
-		      pipe, plane, crtc->base.primary->fb->width,
-		      crtc->base.primary->fb->height,
-		      crtc->base.primary->fb->bits_per_pixel, base,
-		      crtc->base.primary->fb->pitches[0],
-		      plane_config->size);
+		      pipe, plane, fb->width, fb->height, fb->bits_per_pixel,
+		      base, fb->pitches[0], plane_config->size);
+
+	crtc->base.primary->fb = fb;
 }
 
 static bool ironlake_get_pipe_config(struct intel_crtc *crtc,