Re: [PATCH 10/10] drm: Use state helper instead of the plane state pointer
Hi Maxime,
Thank you for the patch.
On Fri, Jan 15, 2021 at 01:57:02PM +0100, Maxime Ripard wrote:
> Many drivers reference the plane->state pointer in order to get the
> current plane state in their atomic_update or atomic_disable hooks,
Please don't use the word "current", it's ambiguous. Do you mean old
state or new state ?
> which would be the new plane state in the global atomic state since
> _swap_state happened when those hooks are run.
Is this relevant ? drm_atomic_helper_swap_state() doesn't change the
old_state and new_state pointers in drm_atomic_state as far as I can
tell.
> 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
>
> @ no_include depends on !include && adds_new_state @
> @@
>
> + #include
> #include
>
> Signed-off-by: Maxime Ripard
> ---
[snip]
> drivers/gpu/drm/omapdrm/omap_plane.c| 6 --
> drivers/gpu/drm/rcar-du/rcar_du_plane.c | 3 ++-
> drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 3 ++-
> drivers/gpu/drm/xlnx/zynqmp_disp.c | 3 ++-
[snip]
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
> b/drivers/gpu/drm/omapdrm/omap_plane.c
> index cd8cf7c786b5..021a94de84a1 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -44,7 +44,8 @@ static void omap_plane_atomic_update(struct drm_plane
> *plane,
> {
> struct omap_drm_private *priv = plane->dev->dev_private;
> struct omap_plane *omap_plane = to_omap_plane(plane);
> - struct drm_plane_state *new_state = plane->state;
This seems to imply that you're interested in the new state.
> + struct drm_plane_state *new_state =
> drm_atomic_get_new_plane_state(state,
> +
> plane);
Does this really make things more obvious ?
> struct omap_overlay_info info;
> int ret;
>
> @@ -89,7 +90,8 @@ static void omap_plane_atomic_update(struct drm_plane
> *plane,
> static void omap_plane_atomic_disable(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);
> struct omap_drm_private *priv = plane->dev->dev_private;
> struct omap_plane *omap_plane = to_omap_plane(plane);
>
[snip]
--
Regards,
Laurent Pinchart
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 09/10] drm/atomic: Pass the full state to planes atomic disable and update
Hi Maxime,
Thank you for the patch.
On Fri, Jan 15, 2021 at 01:57:01PM +0100, Maxime Ripard wrote:
> The current atomic helpers have either their object state being passed as
> an argument or the full atomic state.
>
> The former is the pattern that was done at first, before switching to the
> latter for new hooks or when it was needed.
>
> Let's start convert all the remaining helpers to provide a consistent
s/start convert/convert/ ?
> interface, starting with the planes atomic_update and atomic_disable.
You're not starting anymore, its 09/10 already :-)
> The conversion was done using the coccinelle script below, built tested on
> all the drivers.
>
> @@
> identifier plane, plane_state;
> symbol state;
> @@
>
> struct drm_plane_helper_funcs {
> ...
> void (*atomic_update)(struct drm_plane *plane,
> - struct drm_plane_state *plane_state);
> + struct drm_atomic_state *state);
> ...
> }
>
> @@
> identifier plane, plane_state;
> symbol state;
> @@
>
> struct drm_plane_helper_funcs {
> ...
> void (*atomic_disable)(struct drm_plane *plane,
> -struct drm_plane_state *plane_state);
> +struct drm_atomic_state *state);
> ...
> }
>
> @ plane_atomic_func @
> identifier helpers;
> identifier func;
> @@
>
> (
> static const struct drm_plane_helper_funcs helpers = {
> ...,
> .atomic_update = func,
> ...,
> };
> |
> static const struct drm_plane_helper_funcs helpers = {
> ...,
> .atomic_disable = func,
> ...,
> };
> )
>
> @@
> struct drm_plane_helper_funcs *FUNCS;
> identifier f;
> identifier crtc_state;
> identifier plane, plane_state, state;
> expression e;
> @@
>
> f(struct drm_crtc_state *crtc_state)
> {
> ...
> struct drm_atomic_state *state = e;
> <+...
> (
> - FUNCS->atomic_disable(plane, plane_state)
> + FUNCS->atomic_disable(plane, state)
> |
> - FUNCS->atomic_update(plane, plane_state)
> + FUNCS->atomic_update(plane, state)
> )
> ...+>
> }
>
> @@
> identifier plane_atomic_func.func;
> identifier plane;
> symbol state;
> @@
>
> func(struct drm_plane *plane,
> -struct drm_plane_state *state)
> +struct drm_plane_state *old_plane_state)
> {
> <...
> - state
> + old_plane_state
> ...>
> }
>
> @ ignores_old_state @
> identifier plane_atomic_func.func;
> identifier plane, old_state;
> @@
>
> func(struct drm_plane *plane, struct drm_plane_state *old_state)
> {
> ... when != old_state
> }
>
> @ adds_old_state depends on plane_atomic_func && !ignores_old_state @
> identifier plane_atomic_func.func;
> identifier plane, plane_state;
> @@
>
> func(struct drm_plane *plane, struct drm_plane_state *plane_state)
> {
> + struct drm_plane_state *plane_state =
> drm_atomic_get_old_plane_state(state, plane);
> ...
> }
>
> @ depends on plane_atomic_func @
> identifier plane_atomic_func.func;
> identifier plane, plane_state;
> @@
>
> func(struct drm_plane *plane,
> - struct drm_plane_state *plane_state
> + struct drm_atomic_state *state
> )
> { ... }
>
> @ include depends on adds_old_state @
> @@
>
> #include
>
> @ no_include depends on !include && adds_old_state @
> @@
>
> + #include
> #include
>
> @@
> identifier plane_atomic_func.func;
> identifier plane, state;
> identifier plane_state;
> @@
>
> func(struct drm_plane *plane, struct drm_atomic_state *state) {
> ...
> struct drm_plane_state *plane_state =
> drm_atomic_get_old_plane_state(state, plane);
> <+...
> - plane_state->state
> + state
> ...+>
> }
>
> Signed-off-by: Maxime Ripard
> ---
[snip]
> drivers/gpu/drm/drm_atomic_helper.c | 8
> drivers/gpu/drm/drm_simple_kms_helper.c | 4 +++-
> drivers/gpu/drm/omapdrm/omap_plane.c | 4 ++--
> drivers/gpu/drm/rcar-du/rcar_du_plane.c | 4 +++-
> drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 4 +++-
> include/drm/drm_modeset_helper_vtables.h | 4 ++--
For these,
Reviewed-by: Laurent Pinchart
For drivers/gpu/drm/xlnx/zynqmp_disp.c, please see below.
[snip]
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index e278680b7d5a..39f9e6e76064 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -1166,8 +1166,10 @@ zynqmp_disp_plane_atomic_check(struct drm_plane *plane,
>
> static void
> zynqmp_disp_plane_atomic_disable(struct drm_plane *plane,
> - struct drm_plane_state *old_state)
> + struct drm_atomic_state *state)
> {
> + struct drm_plane_state *old_state =
> drm_atomic_get_old_plane_state(state,
> +
> plane);
> struct zynqmp_disp_layer *layer = plane_to_layer(plane);
>
>
Re: [PATCH 07/10] drm: Store new plane state in a variable for atomic_update and disable
Hi Maxime,
Thank you for the patch.
On Fri, Jan 15, 2021 at 01:56:59PM +0100, Maxime Ripard wrote:
> In order to store the new plane state in a subsequent helper, let's move
> the plane->state dereferences into a variable.
>
> This was done using the following coccinelle script, plus some hand
> changes for vmwgfx:
>
> @ 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,
> ...,
> };
> )
>
> @ has_new_state_old_state @
> identifier plane_atomic_func.func;
> identifier plane;
> identifier new_state;
> symbol old_state;
> @@
>
> func(struct drm_plane *plane, struct drm_plane_state *old_state)
> {
> ...
> struct drm_plane_state *new_state = plane->state;
> ...
> }
>
> @ depends on !has_new_state_old_state @
> identifier plane_atomic_func.func;
> identifier plane;
> symbol old_state;
> @@
>
> func(struct drm_plane *plane, struct drm_plane_state *old_state)
> {
> + struct drm_plane_state *new_state = plane->state;
> <+...
> - plane->state
> + new_state
> ...+>
> }
>
> @ has_new_state_state @
> identifier plane_atomic_func.func;
> identifier plane;
> identifier new_state;
> symbol state;
> @@
>
> func(struct drm_plane *plane, struct drm_plane_state *state)
> {
> ...
> struct drm_plane_state *new_state = plane->state;
> ...
> }
>
> @ depends on !has_new_state_state @
> identifier plane_atomic_func.func;
> identifier plane;
> symbol state;
> @@
>
> func(struct drm_plane *plane, struct drm_plane_state *state)
> {
> + struct drm_plane_state *new_plane_state = plane->state;
> <+...
> - plane->state
> + new_plane_state
> ...+>
> }
>
> @ has_new_state_old_s @
> identifier plane_atomic_func.func;
> identifier plane;
> identifier new_state;
> symbol old_s;
> @@
>
> func(struct drm_plane *plane, struct drm_plane_state *old_s)
> {
> ...
> struct drm_plane_state *new_state = plane->state;
> ...
> }
>
> @ depends on !has_new_state_old_s @
> identifier plane_atomic_func.func;
> identifier plane;
> symbol old_s;
> @@
>
> func(struct drm_plane *plane, struct drm_plane_state *old_s)
> {
> + struct drm_plane_state *new_s = plane->state;
> <+...
> - plane->state
> + new_s
> ...+>
> }
I may have taken this as an opportunity to align naming conventions for
variables across drivers, but that may just be me.
>
> Signed-off-by: Maxime Ripard
> ---
[snip]
> drivers/gpu/drm/omapdrm/omap_plane.c | 5 ++-
> drivers/gpu/drm/rcar-du/rcar_du_plane.c | 5 ++-
> drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 3 +-
> drivers/gpu/drm/xlnx/zynqmp_disp.c| 7 ++--
For these, with the small issue below addressed,
Reviewed-by: Laurent Pinchart
[snip]
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
> b/drivers/gpu/drm/omapdrm/omap_plane.c
> index 1042e1147e74..de5ad69af4cb 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -88,11 +88,12 @@ static void omap_plane_atomic_update(struct drm_plane
> *plane,
> static void omap_plane_atomic_disable(struct drm_plane *plane,
> struct drm_plane_state *old_state)
> {
> + struct drm_plane_state *new_state = plane->state;
> struct omap_drm_private *priv = plane->dev->dev_private;
> struct omap_plane *omap_plane = to_omap_plane(plane);
>
> - plane->state->rotation = DRM_MODE_ROTATE_0;
> - plane->state->zpos = plane->type == DRM_PLANE_TYPE_PRIMARY
> + new_state->rotation = DRM_MODE_ROTATE_0;
> + new_state->zpos = plane->type == DRM_PLANE_TYPE_PRIMARY
> ? 0 : omap_plane->id;
Can you fix the indentation ?
> dispc_ovl_enable(priv->dispc, omap_plane->id, false);
[snip]
--
Regards,
Laurent Pinchart
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 05/10] drm: Use the state pointer directly in planes atomic_check
Hi Maxime,
Thank you for the patch.
On Fri, Jan 15, 2021 at 01:56:57PM +0100, Maxime Ripard wrote:
> Now that atomic_check takes the global atomic state as a parameter, we
> don't need to go through the pointer in the plane state.
>
> This was done using the following coccinelle script:
>
> @ plane_atomic_func @
> identifier helpers;
> identifier func;
> @@
>
> static struct drm_plane_helper_funcs helpers = {
> ...,
> .atomic_check = func,
> ...,
> };
>
> @@
> identifier plane_atomic_func.func;
> identifier plane, state;
> identifier plane_state;
> @@
>
> func(struct drm_plane *plane, struct drm_atomic_state *state) {
> ...
> - struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state,
> plane);
> <... when != plane_state
> - plane_state->state
> + state
> ...>
> }
>
> @@
> identifier plane_atomic_func.func;
> identifier plane, state;
> identifier plane_state;
> @@
>
> func(struct drm_plane *plane, struct drm_atomic_state *state) {
> ...
> struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state,
> plane);
> <...
> - plane_state->state
> + state
> ...>
> }
>
> Signed-off-by: Maxime Ripard
> ---
[snip]
> drivers/gpu/drm/omapdrm/omap_plane.c | 2 +-
> drivers/gpu/drm/xlnx/zynqmp_disp.c| 2 +-
For these, with the small issue below addressed,
Reviewed-by: Laurent Pinchart
[snip]
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index b0a3ba528718..d749acc78c85 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -1152,7 +1152,7 @@ zynqmp_disp_plane_atomic_check(struct drm_plane *plane,
> if (!new_plane_state->crtc)
> return 0;
>
> - crtc_state = drm_atomic_get_crtc_state(new_plane_state->state,
> + crtc_state = drm_atomic_get_crtc_state(state,
> new_plane_state->crtc);
This now holds on a single line.
> if (IS_ERR(crtc_state))
> return PTR_ERR(crtc_state);
[snip]
--
Regards,
Laurent Pinchart
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 04/10] drm/atomic: Pass the full state to planes atomic_check
Hi Maxime,
Thank you for the patch.
On Fri, Jan 15, 2021 at 01:56:56PM +0100, Maxime Ripard wrote:
> The current atomic helpers have either their object state being passed as
> an argument or the full atomic state.
>
> The former is the pattern that was done at first, before switching to the
> latter for new hooks or when it was needed.
>
> Let's start convert all the remaining helpers to provide a consistent
s/start convert/convert/ ?
> interface, starting with the planes atomic_check.
>
> The conversion was done using the coccinelle script below plus some
> manual changes for vmwgfx, built tested on all the drivers.
>
> @@
> identifier plane, plane_state;
> symbol state;
> @@
>
> struct drm_plane_helper_funcs {
> ...
> int (*atomic_check)(struct drm_plane *plane,
> - struct drm_plane_state *plane_state);
> + struct drm_atomic_state *state);
> ...
> }
>
> @ plane_atomic_func @
> identifier helpers;
> identifier func;
> @@
>
> static const struct drm_plane_helper_funcs helpers = {
> ...,
> .atomic_check = func,
> ...,
> };
>
> @@
> struct drm_plane_helper_funcs *FUNCS;
> identifier f;
> identifier dev;
> identifier plane, plane_state, state;
> @@
>
> f(struct drm_device *dev, struct drm_atomic_state *state)
> {
> <+...
> - FUNCS->atomic_check(plane, plane_state)
> + FUNCS->atomic_check(plane, state)
> ...+>
> }
>
> @@
> identifier plane_atomic_func.func;
> identifier plane;
> symbol state;
> @@
>
> func(struct drm_plane *plane,
> -struct drm_plane_state *state)
> +struct drm_plane_state *new_plane_state)
> {
> <...
> - state
> + new_plane_state
> ...>
> }
>
Is this needed, or is it a leftover from 02/10 ?
> @ ignores_new_state @
> identifier plane_atomic_func.func;
> identifier plane, new_plane_state;
> @@
>
> func(struct drm_plane *plane, struct drm_plane_state *new_plane_state)
> {
> ... when != new_plane_state
> }
>
> @ adds_new_state depends on plane_atomic_func && !ignores_new_state @
> identifier plane_atomic_func.func;
> identifier plane, new_plane_state;
> @@
>
> func(struct drm_plane *plane, struct drm_plane_state *new_plane_state)
> {
> + struct drm_plane_state *new_plane_state =
> drm_atomic_get_new_plane_state(state, plane);
> ...
> }
>
> @ depends on plane_atomic_func @
> identifier plane_atomic_func.func;
> identifier plane, new_plane_state;
> @@
>
> func(struct drm_plane *plane,
> - struct drm_plane_state *new_plane_state
> + struct drm_atomic_state *state
> )
> { ... }
>
> @ include depends on adds_new_state @
> @@
>
> #include
>
> @ no_include depends on !include && adds_new_state @
> @@
>
> + #include
> #include
>
> Signed-off-by: Maxime Ripard
> ---
[snip]
> drivers/gpu/drm/drm_atomic_helper.c | 2 +-
> drivers/gpu/drm/drm_simple_kms_helper.c | 4 +++-
> drivers/gpu/drm/omapdrm/omap_plane.c | 4 +++-
> drivers/gpu/drm/rcar-du/rcar_du_plane.c | 4 +++-
> drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 5 -
> drivers/gpu/drm/xlnx/zynqmp_disp.c| 4 +++-
For these,
Reviewed-by: Laurent Pinchart
[snip]
--
Regards,
Laurent Pinchart
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 02/10] drm: Rename plane atomic_check state names
Hi Maxime,
Thank you for the patch.
On Fri, Jan 15, 2021 at 01:56:54PM +0100, Maxime Ripard wrote:
> Most drivers call the argument to the plane atomic_check hook simply
> state, which is going to conflict with the global atomic state in a
> later rework. Let's rename it to new_plane_state (or new_state depending
> on the convention used in the driver).
>
> This was done using the coccinelle script below, and built tested:
>
> @ plane_atomic_func @
> identifier helpers;
> identifier func;
> @@
>
> static const struct drm_plane_helper_funcs helpers = {
> .atomic_check = func,
> };
>
> @ has_old_state @
> identifier plane_atomic_func.func;
> identifier plane;
> expression e;
> symbol old_state;
> symbol state;
> @@
>
> func(struct drm_plane *plane, struct drm_plane_state *state)
> {
> ...
> struct drm_plane_state *old_state = e;
> ...
> }
>
> @ depends on has_old_state @
> identifier plane_atomic_func.func;
> identifier plane;
> symbol old_state;
> @@
>
> func(struct drm_plane *plane,
> - struct drm_plane_state *state
> + struct drm_plane_state *new_state
> )
> {
> <+...
> - state
> + new_state
> ...+>
> }
>
> @ has_state @
> identifier plane_atomic_func.func;
> identifier plane;
> symbol state;
> @@
>
> func(struct drm_plane *plane, struct drm_plane_state *state)
> {
> ...
> }
>
> @ depends on has_state @
> identifier plane_atomic_func.func;
> identifier plane;
> symbol old_state;
> @@
>
> func(struct drm_plane *plane,
> - struct drm_plane_state *state
> + struct drm_plane_state *new_plane_state
> )
> {
> <+...
> - state
> + new_plane_state
> ...+>
> }
>
> Signed-off-by: Maxime Ripard
> ---
[...]
> drivers/gpu/drm/omapdrm/omap_plane.c | 19 +
> drivers/gpu/drm/rcar-du/rcar_du_plane.c | 7 ++--
> drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 7 ++--
> drivers/gpu/drm/xlnx/zynqmp_disp.c| 10 +++--
For these, with the comment below addressed,
Reviewed-by: Laurent Pinchart
> 41 files changed, 402 insertions(+), 357 deletions(-)
[snip]
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
> b/drivers/gpu/drm/omapdrm/omap_plane.c
> index 51dc24acea73..78d0eb1fd69d 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -99,18 +99,19 @@ static void omap_plane_atomic_disable(struct drm_plane
> *plane,
> }
>
> static int omap_plane_atomic_check(struct drm_plane *plane,
> -struct drm_plane_state *state)
> +struct drm_plane_state *new_plane_state)
> {
> struct drm_crtc_state *crtc_state;
>
> - if (!state->fb)
> + if (!new_plane_state->fb)
> return 0;
>
> /* crtc should only be NULL when disabling (i.e., !state->fb) */
s/state/new_plane_state/ here too ?
> - if (WARN_ON(!state->crtc))
> + if (WARN_ON(!new_plane_state->crtc))
> return 0;
>
> - crtc_state = drm_atomic_get_existing_crtc_state(state->state,
> state->crtc);
> + crtc_state = drm_atomic_get_existing_crtc_state(new_plane_state->state,
> + new_plane_state->crtc);
> /* we should have a crtc state if the plane is attached to a crtc */
> if (WARN_ON(!crtc_state))
> return 0;
> @@ -118,17 +119,17 @@ static int omap_plane_atomic_check(struct drm_plane
> *plane,
> if (!crtc_state->enable)
> return 0;
>
> - if (state->crtc_x < 0 || state->crtc_y < 0)
> + if (new_plane_state->crtc_x < 0 || new_plane_state->crtc_y < 0)
> return -EINVAL;
>
> - if (state->crtc_x + state->crtc_w > crtc_state->adjusted_mode.hdisplay)
> + if (new_plane_state->crtc_x + new_plane_state->crtc_w >
> crtc_state->adjusted_mode.hdisplay)
I can't help thinking we're using too long variable names... :-(
> return -EINVAL;
>
> - if (state->crtc_y + state->crtc_h > crtc_state->adjusted_mode.vdisplay)
> + if (new_plane_state->crtc_y + new_plane_state->crtc_h >
> crtc_state->adjusted_mode.vdisplay)
> return -EINVAL;
>
> - if (state->rotation != DRM_MODE_ROTATE_0 &&
> - !omap_framebuffer_supports_rotation(state->fb))
> + if (new_plane_state->rotation != DRM_MODE_ROTATE_0 &&
> + !omap_framebuffer_supports_rotation(new_plane_state->fb))
> return -EINVAL;
>
> return 0;
[...]
--
Regards,
Laurent Pinchart
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH net-next v7] vhost_net: avoid tx queue stuck when sendmsg fails
On Fri, Jan 15, 2021 at 1:12 AM Jason Wang wrote: > > > On 2021/1/15 下午12:46, wangyunjian wrote: > > From: Yunjian Wang > > > > Currently the driver doesn't drop a packet which can't be sent by tun > > (e.g bad packet). In this case, the driver will always process the > > same packet lead to the tx queue stuck. > > > > To fix this issue: > > 1. in the case of persistent failure (e.g bad packet), the driver > > can skip this descriptor by ignoring the error. > > 2. in the case of transient failure (e.g -ENOBUFS, -EAGAIN and -ENOMEM), > > the driver schedules the worker to try again. > > > > Signed-off-by: Yunjian Wang > > > Acked-by: Jason Wang Acked-by: Willem de Bruijn ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 02/10] drm: Rename plane atomic_check state names
Hi
Am 15.01.21 um 13:56 schrieb Maxime Ripard:
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c
b/drivers/gpu/drm/imx/ipuv3-plane.c
index 8a4235d9d9f1..2cb09e9d9306 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -344,12 +344,12 @@ static const struct drm_plane_funcs ipu_plane_funcs = {
};
static int ipu_plane_atomic_check(struct drm_plane *plane,
- struct drm_plane_state *state)
+ struct drm_plane_state *new_state)
It's not 'new_plane_state' ?
Best regards
Thomas
{
struct drm_plane_state *old_state = plane->state;
struct drm_crtc_state *crtc_state;
struct device *dev = plane->dev->dev;
- struct drm_framebuffer *fb = state->fb;
+ struct drm_framebuffer *fb = new_state->fb;
struct drm_framebuffer *old_fb = old_state->fb;
unsigned long eba, ubo, vbo, old_ubo, old_vbo, alpha_eba;
bool can_position = (plane->type == DRM_PLANE_TYPE_OVERLAY);
@@ -359,15 +359,16 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
if (!fb)
return 0;
- if (WARN_ON(!state->crtc))
+ if (WARN_ON(!new_state->crtc))
return -EINVAL;
crtc_state =
- drm_atomic_get_existing_crtc_state(state->state, state->crtc);
+ drm_atomic_get_existing_crtc_state(new_state->state,
+ new_state->crtc);
if (WARN_ON(!crtc_state))
return -EINVAL;
- ret = drm_atomic_helper_check_plane_state(state, crtc_state,
+ ret = drm_atomic_helper_check_plane_state(new_state, crtc_state,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
can_position, true);
@@ -381,7 +382,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
switch (plane->type) {
case DRM_PLANE_TYPE_PRIMARY:
/* full plane minimum width is 13 pixels */
- if (drm_rect_width(>dst) < 13)
+ if (drm_rect_width(_state->dst) < 13)
return -EINVAL;
break;
case DRM_PLANE_TYPE_OVERLAY:
@@ -391,7 +392,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
return -EINVAL;
}
- if (drm_rect_height(>dst) < 2)
+ if (drm_rect_height(_state->dst) < 2)
return -EINVAL;
/*
@@ -402,12 +403,12 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
* callback.
*/
if (old_fb &&
- (drm_rect_width(>dst) != drm_rect_width(_state->dst) ||
-drm_rect_height(>dst) != drm_rect_height(_state->dst) ||
+ (drm_rect_width(_state->dst) != drm_rect_width(_state->dst)
||
+drm_rect_height(_state->dst) !=
drm_rect_height(_state->dst) ||
fb->format != old_fb->format))
crtc_state->mode_changed = true;
- eba = drm_plane_state_to_eba(state, 0);
+ eba = drm_plane_state_to_eba(new_state, 0);
if (eba & 0x7)
return -EINVAL;
@@ -433,7 +434,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
* - Only EBA may be changed while scanout is active
* - The strides of U and V planes must be identical.
*/
- vbo = drm_plane_state_to_vbo(state);
+ vbo = drm_plane_state_to_vbo(new_state);
if (vbo & 0x7 || vbo > 0xf8)
return -EINVAL;
@@ -450,7 +451,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
fallthrough;
case DRM_FORMAT_NV12:
case DRM_FORMAT_NV16:
- ubo = drm_plane_state_to_ubo(state);
+ ubo = drm_plane_state_to_ubo(new_state);
if (ubo & 0x7 || ubo > 0xf8)
return -EINVAL;
@@ -471,8 +472,8 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
* The x/y offsets must be even in case of horizontal/vertical
* chroma subsampling.
*/
- if (((state->src.x1 >> 16) & (fb->format->hsub - 1)) ||
- ((state->src.y1 >> 16) & (fb->format->vsub - 1)))
+ if (((new_state->src.x1 >> 16) & (fb->format->hsub - 1)) ||
+ ((new_state->src.y1 >> 16) & (fb->format->vsub - 1)))
return -EINVAL;
break;
case DRM_FORMAT_RGB565_A8:
@@ -481,7 +482,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
case DRM_FORMAT_BGR888_A8:
case DRM_FORMAT_RGBX_A8:
case DRM_FORMAT_BGRX_A8:
- alpha_eba = drm_plane_state_to_eba(state, 1);
+ alpha_eba = drm_plane_state_to_eba(new_state, 1);
if
Re: Change eats memory on my server
(cc'ing dri-devel)
Hi
Am 14.01.21 um 16:15 schrieb Eli Cohen:
Hi Thomas,
After long bisecting I found that this patch,
commit 1086db71a1dbbfb32ffb42cf0d540b69956f951e
Author: Thomas Zimmermann
Date: Tue Nov 3 10:30:06 2020 +0100
drm/vram-helper: Remove invariant parameters from internal kmap function
is the offending patch causing the kernel to eat my server memory. It
will eat all 24 GB of ram after around 7 hours.
Thanks for reporting. It's a bit strange that this commit shows up,
because there's nothing that should produce a memory leak.
Could you please double-check that 3fb91f56aea4 ("drm/udl: Retrieve USB
device from struct drm_device.dev") works correctly and that
823efa922102 ("drm/cma-helper: Remove empty drm_gem_cma_prime_vunmap()")
is broken?
For one of the broken commits, could you please send us the output of
dmesg | grep -i drm
after most of the memory got leaked?
Best regards
Thomas
It's a a super micro server. The output of dmidecode is below:
# dmidecode 3.2
Getting SMBIOS data from sysfs.
SMBIOS 3.1.1 present.
Table at 0x6F01B000.
Handle 0x, DMI type 0, 26 bytes
BIOS Information
Vendor: American Megatrends Inc.
Version: 2.0
Release Date: 11/30/2017
Address: 0xF
Runtime Size: 64 kB
ROM Size: 32 MB
Characteristics:
PCI is supported
BIOS is upgradeable
BIOS shadowing is allowed
Boot from CD is supported
Selectable boot is supported
BIOS ROM is socketed
EDD is supported
5.25"/1.2 MB floppy services are supported (int 13h)
3.5"/720 kB floppy services are supported (int 13h)
3.5"/2.88 MB floppy services are supported (int 13h)
Print screen service is supported (int 5h)
Serial services are supported (int 14h)
Printer services are supported (int 17h)
ACPI is supported
USB legacy is supported
BIOS boot specification is supported
Targeted content distribution is supported
UEFI is supported
BIOS Revision: 5.12
Handle 0x0001, DMI type 1, 27 bytes
System Information
Manufacturer: Supermicro
Product Name: Super Server
Version: 0123456789
Serial Number: 0123456789
UUID: ----0cc47af973ca
Wake-up Type: Power Switch
SKU Number: To be filled by O.E.M.
Family: To be filled by O.E.M.
Handle 0x0002, DMI type 2, 15 bytes
Base Board Information
Manufacturer: Supermicro
Product Name: X11DPT-B
Version: 1.02
Serial Number: HM179S003332
Asset Tag: To be filled by O.E.M.
Features:
Board is a hosting board
Board is replaceable
Location In Chassis: To be filled by O.E.M.
Chassis Handle: 0x0003
Type: Motherboard
Contained Object Handles: 0
Handle 0x0003, DMI type 3, 22 bytes
Chassis Information
Manufacturer: Supermicro
Type: Main Server Chassis
Lock: Not Present
Version: 0123456789
Serial Number: 0123456789
Asset Tag: To be filled by O.E.M.
Boot-up State: Safe
Power Supply State: Safe
Thermal State: Safe
Security Status: None
OEM Information: 0x
Height: Unspecified
Number Of Power Cords: 1
Contained Elements: 0
SKU Number: To be filled by O.E.M.
Handle 0x0004, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: JVGA1
Internal Connector Type: None
External Reference Designator: VGA
External Connector Type: DB-15 female
Port Type: Video Port
Handle 0x0005, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: JLAN1
Internal Connector Type: None
External Reference Designator: IPMI_LAN
External Connector Type: RJ-45
Port Type: Network Port
Handle 0x0006, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: JUSB1
Internal Connector Type: None
External Reference Designator: USB0/1(3.0)
External Connector Type: Access Bus (USB)
Port Type: USB
Handle 0x0007, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: TPM/PORT80
Internal Connector Type: Other
External Reference Designator: Not Specified
External Connector Type: None
Port Type: Other
Handle 0x0008, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: FAN3
Internal Connector Type: Other
External Reference Designator: Not Specified
External Connector Type: None
Port Type: Other
Handle 0x0009, DMI type 8, 9 bytes
Port Connector
