Trimming Cc list way down, sorry if that's too much.
Quoting Maxime Ripard (2021-02-19 04:00:30)
Many drivers reference the plane->state pointer in order to get the current plane state in their atomic_update or atomic_disable hooks, which would be the new plane state in the global atomic state since _swap_state happened when those hooks are run.
Does this mean drm_atomic_helper_swap_state()?
Use the drm_atomic_get_new_plane_state helper to get that state to make it more obvious.
This was made using the coccinelle script below:
@ plane_atomic_func @ identifier helpers; identifier func; @@
( static const struct drm_plane_helper_funcs helpers = { ..., .atomic_disable = func, ..., }; | static const struct drm_plane_helper_funcs helpers = { ..., .atomic_update = func, ..., }; )
@ adds_new_state @ identifier plane_atomic_func.func; identifier plane, state; identifier new_state; @@
func(struct drm_plane *plane, struct drm_atomic_state *state) { ...
struct drm_plane_state *new_state = plane->state;
struct drm_plane_state *new_state = drm_atomic_get_new_plane_state(state, plane); ...}
@ include depends on adds_new_state @ @@
#include <drm/drm_atomic.h>
@ no_include depends on !include && adds_new_state @ @@
- #include <drm/drm_atomic.h> #include <drm/...>
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 3 ++- drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 4 +++- drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 3 ++- diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 31071f9e21d7..e8ce72fe54a4 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -1244,7 +1244,8 @@ static void dpu_plane_atomic_update(struct drm_plane *plane, struct drm_atomic_state *state) { struct dpu_plane *pdpu = to_dpu_plane(plane);
struct drm_plane_state *new_state = plane->state;
struct drm_plane_state *new_state = drm_atomic_get_new_plane_state(state,plane); pdpu->is_error = false;
This is oopsing for me. It turns out that 'new_state' is NULL. According to the comments drm_atomic_get_new_plane_state() can return NULL if the plane isn't part of the global state. I haven't looked much further but wanted to report it here in case that type of return value makes sense.
If I revert this patch from linux-next my display works and doesn't crash the system. Or I can check for NULL in the if below and it also works.
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index df7f3d3afd8b..f31b89531f6a 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -1251,7 +1251,7 @@ static void dpu_plane_atomic_update(struct drm_plane *plane,
DPU_DEBUG_PLANE(pdpu, "\n");
- if (!new_state->visible) { + if (new_state && !new_state->visible) { _dpu_plane_atomic_disable(plane); } else { dpu_plane_sspp_atomic_update(plane);