Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 321ebf04 authored by Daniel Vetter's avatar Daniel Vetter
Browse files

drm/atomic: Refcounting for plane_state->fb 



So my original plan was that the drm core refcounts framebuffers like
with the legacy ioctls. But that doesn't work for a bunch of reasons:

- State objects might live longer than until the next fb change
  happens for a plane. For example delayed cleanup work only happens
  _after_ the pageflip ioctl has completed. So this definitely doesn't
  work without the plane state holding its own references.

- The other issue is transition from legacy to atomic implementations,
  where the driver works under a mix of both worlds. Which means
  legacy paths might not properly update the ->fb pointer under
  plane->state->fb. Which is a bit a problem when then someone comes
  around and _does_ try to clean it up when it's long gone.

The second issue is just a bit a transition bug, since drivers should
update plane->state->fb in all the paths that aren't converted yet.
But a bit more robustness for the transition can't hurt - we pull
similar tricks with cleaning up the old fb in the transitional helpers
already.

The pattern for drivers that transition is

	if (plane->state)
		drm_atomic_set_fb_for_plane(plane->state, plane->fb);

inserted after the fb update has logically completed at the end of
->set_config (or ->set_base/mode_set if using the crtc helpers),
->page_flip, ->update_plane or any other entry point which updates
plane->fb.

v2: Update kerneldoc - copypasta fail.

v3: Fix spelling in the commit message (Sean).

Reviewed-by: default avatarSean Paul <seanpaul@chromium.org>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
parent 3150c7d0

drivers/gpu/drm/drm_atomic.c

+28 −0
Original line number Diff line number Diff line
@@ -367,6 +367,34 @@ drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state, 
}

EXPORT_SYMBOL(drm_atomic_set_crtc_for_plane);



/**

 * drm_atomic_set_fb_for_plane - set crtc for plane

 * @plane_state: atomic state object for the plane

 * @fb: fb to use for the plane

 *

 * Changing the assigned framebuffer for a plane requires us to grab a reference

 * to the new fb and drop the reference to the old fb, if there is one. This

 * function takes care of all these details besides updating the pointer in the

 * state object itself.

 */

void

drm_atomic_set_fb_for_plane(struct drm_plane_state *plane_state,

			    struct drm_framebuffer *fb)

{

	if (plane_state->fb)

		drm_framebuffer_unreference(plane_state->fb);

	if (fb)

		drm_framebuffer_reference(fb);

	plane_state->fb = fb;



	if (fb)

		DRM_DEBUG_KMS("Set [FB:%d] for plane state %p\n",

			      fb->base.id, plane_state);

	else

		DRM_DEBUG_KMS("Set [NOFB] for plane state %p\n", plane_state);

}

EXPORT_SYMBOL(drm_atomic_set_fb_for_plane);



/**

 * drm_atomic_set_crtc_for_connector - set crtc for connector

 * @conn_state: atomic state object for the connector

drivers/gpu/drm/drm_atomic_helper.c

+19 −6
Original line number Diff line number Diff line
@@ -1182,7 +1182,7 @@ retry: 
	ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);

	if (ret != 0)

		goto fail;

	plane_state->fb = fb;

	drm_atomic_set_fb_for_plane(plane_state, fb);

	plane_state->crtc_x = crtc_x;

	plane_state->crtc_y = crtc_y;

	plane_state->crtc_h = crtc_h;
@@ -1250,7 +1250,7 @@ retry: 
	ret = drm_atomic_set_crtc_for_plane(plane_state, NULL);

	if (ret != 0)

		goto fail;

	plane_state->fb = NULL;

	drm_atomic_set_fb_for_plane(plane_state, NULL);

	plane_state->crtc_x = 0;

	plane_state->crtc_y = 0;

	plane_state->crtc_h = 0;
@@ -1422,7 +1422,7 @@ retry: 
	ret = drm_atomic_set_crtc_for_plane(primary_state, crtc);

	if (ret != 0)

		goto fail;

	primary_state->fb = set->fb;

	drm_atomic_set_fb_for_plane(primary_state, set->fb);

	primary_state->crtc_x = 0;

	primary_state->crtc_y = 0;

	primary_state->crtc_h = set->mode->vdisplay;
@@ -1694,7 +1694,7 @@ retry: 
	ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);

	if (ret != 0)

		goto fail;

	plane_state->fb = fb;

	drm_atomic_set_fb_for_plane(plane_state, fb);



	ret = drm_atomic_async_commit(state);

	if (ret != 0)
@@ -1808,6 +1808,9 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state); 
 */

void drm_atomic_helper_plane_reset(struct drm_plane *plane)

{

	if (plane->state && plane->state->fb)

		drm_framebuffer_unreference(plane->state->fb);



	kfree(plane->state);

	plane->state = kzalloc(sizeof(*plane->state), GFP_KERNEL);

}
@@ -1823,10 +1826,17 @@ EXPORT_SYMBOL(drm_atomic_helper_plane_reset); 
struct drm_plane_state *

drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane)

{

	struct drm_plane_state *state;



	if (WARN_ON(!plane->state))

		return NULL;



	return kmemdup(plane->state, sizeof(*plane->state), GFP_KERNEL);

	state = kmemdup(plane->state, sizeof(*plane->state), GFP_KERNEL);



	if (state && state->fb)

		drm_framebuffer_reference(state->fb);



	return state;

}

EXPORT_SYMBOL(drm_atomic_helper_plane_duplicate_state);
@@ -1841,6 +1851,9 @@ EXPORT_SYMBOL(drm_atomic_helper_plane_duplicate_state); 
void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane,

					   struct drm_plane_state *state)

{

	if (state->fb)

		drm_framebuffer_unreference(state->fb);



	kfree(state);

}

EXPORT_SYMBOL(drm_atomic_helper_plane_destroy_state);

drivers/gpu/drm/drm_crtc_helper.c

+4 −3
Original line number Diff line number Diff line
@@ -34,11 +34,13 @@ 
#include <linux/moduleparam.h>



#include <drm/drmP.h>

#include <drm/drm_atomic.h>

#include <drm/drm_crtc.h>

#include <drm/drm_fourcc.h>

#include <drm/drm_crtc_helper.h>

#include <drm/drm_fb_helper.h>

#include <drm/drm_plane_helper.h>

#include <drm/drm_atomic_helper.h>

#include <drm/drm_edid.h>



/**
@@ -998,15 +1000,14 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, 
	if (plane->funcs->atomic_duplicate_state)

		plane_state = plane->funcs->atomic_duplicate_state(plane);

	else if (plane->state)

		plane_state = kmemdup(plane->state, sizeof(*plane_state),

				      GFP_KERNEL);

		plane_state = drm_atomic_helper_plane_duplicate_state(plane);

	else

		plane_state = kzalloc(sizeof(*plane_state), GFP_KERNEL);

	if (!plane_state)

		return -ENOMEM;



	plane_state->crtc = crtc;

	plane_state->fb = crtc->primary->fb;

	drm_atomic_set_fb_for_plane(plane_state, crtc->primary->fb);

	plane_state->crtc_x = 0;

	plane_state->crtc_y = 0;

	plane_state->crtc_h = crtc->mode.vdisplay;

drivers/gpu/drm/drm_plane_helper.c

+7 −7
Original line number Diff line number Diff line
@@ -27,7 +27,9 @@ 
#include <drm/drmP.h>

#include <drm/drm_plane_helper.h>

#include <drm/drm_rect.h>

#include <drm/drm_atomic.h>

#include <drm/drm_crtc_helper.h>

#include <drm/drm_atomic_helper.h>



#define SUBPIXEL_MASK 0xffff
@@ -464,7 +466,7 @@ out: 
		if (plane->funcs->atomic_destroy_state)

			plane->funcs->atomic_destroy_state(plane, plane_state);

		else

			kfree(plane_state);

			drm_atomic_helper_plane_destroy_state(plane, plane_state);

	}



	return ret;
@@ -505,15 +507,14 @@ int drm_plane_helper_update(struct drm_plane *plane, struct drm_crtc *crtc, 
	if (plane->funcs->atomic_duplicate_state)

		plane_state = plane->funcs->atomic_duplicate_state(plane);

	else if (plane->state)

		plane_state = kmemdup(plane->state, sizeof(*plane_state),

				      GFP_KERNEL);

		plane_state = drm_atomic_helper_plane_duplicate_state(plane);

	else

		plane_state = kzalloc(sizeof(*plane_state), GFP_KERNEL);

	if (!plane_state)

		return -ENOMEM;



	plane_state->crtc = crtc;

	plane_state->fb = fb;

	drm_atomic_set_fb_for_plane(plane_state, fb);

	plane_state->crtc_x = crtc_x;

	plane_state->crtc_y = crtc_y;

	plane_state->crtc_h = crtc_h;
@@ -552,15 +553,14 @@ int drm_plane_helper_disable(struct drm_plane *plane) 
	if (plane->funcs->atomic_duplicate_state)

		plane_state = plane->funcs->atomic_duplicate_state(plane);

	else if (plane->state)

		plane_state = kmemdup(plane->state, sizeof(*plane_state),

				      GFP_KERNEL);

		plane_state = drm_atomic_helper_plane_duplicate_state(plane);

	else

		plane_state = kzalloc(sizeof(*plane_state), GFP_KERNEL);

	if (!plane_state)

		return -ENOMEM;



	plane_state->crtc = NULL;

	plane_state->fb = NULL;

	drm_atomic_set_fb_for_plane(plane_state, NULL);



	return drm_plane_helper_commit(plane, plane_state, plane->fb);

}

include/drm/drm_atomic.h

+2 −0
Original line number Diff line number Diff line
@@ -46,6 +46,8 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state, 
int __must_check

drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state,

			      struct drm_crtc *crtc);

void drm_atomic_set_fb_for_plane(struct drm_plane_state *plane_state,

				 struct drm_framebuffer *fb);

int __must_check

drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,

				  struct drm_crtc *crtc);