Hi,
On Wednesday, September 20, 2023 6:31 PM Lucas Stach <l.stach@xxxxxxxxxxxxxx> wrote:
> Force a modeset if the new FB has a different format than the
> currently active one. While it might be possible to change between
> compatible formats without a full modeset as the format control is
> also supposed to be double buffered, the colorspace conversion is
> not, so when the CSC changes we need a modeset.
>
> For now just always force a modeset when the FB format changes to
> properly reconfigure all parts of the device for the new format.
>
> Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/mxsfb/lcdif_drv.c | 18 +++++++++++++++++-
> drivers/gpu/drm/mxsfb/lcdif_kms.c | 26 ++++++++++++++++++++------
> 2 files changed, 37 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c
> b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> index 18de2f17e249..b74f0cf1e240 100644
> --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
> +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> @@ -30,9 +30,25 @@
> #include "lcdif_drv.h"
> #include "lcdif_regs.h"
>
> +static int lcdif_atomic_check(struct drm_device *dev,
> + struct drm_atomic_state *state)
" checkpatch.pl --strict" complains:
CHECK: Alignment should match open parenthesis
#31: FILE: drivers/gpu/drm/mxsfb/lcdif_drv.c:34:
+static int lcdif_atomic_check(struct drm_device *dev,
+ struct drm_atomic_state *state)
> +{
> + int ret;
> +
> + ret = drm_atomic_helper_check(dev, state);
> + if (ret)
> + return ret;
> +
> + /*
> + * Check modeset again in case crtc_state->mode_changed is
> + * updated in plane's ->atomic_check callback.
> + */
> + return drm_atomic_helper_check_modeset(dev, state);
This additional check looks fine, but it's done for every commit.
Is it ok to move it to lcdif_plane_atomic_check() where mode_changed
is set for those commits in question?
Regards,
Liu Ying
> +}
> +
> static const struct drm_mode_config_funcs lcdif_mode_config_funcs = {
> .fb_create = drm_gem_fb_create,
> - .atomic_check = drm_atomic_helper_check,
> + .atomic_check = lcdif_atomic_check,
> .atomic_commit = drm_atomic_helper_commit,
> };
>
> diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c
> b/drivers/gpu/drm/mxsfb/lcdif_kms.c
> index 3ebf55d06027..8167604bd3f8 100644
> --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c
> +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c
> @@ -647,18 +647,32 @@ static const struct drm_crtc_funcs lcdif_crtc_funcs
> = {
> static int lcdif_plane_atomic_check(struct drm_plane *plane,
> struct drm_atomic_state *state)
> {
> - struct drm_plane_state *plane_state =
> drm_atomic_get_new_plane_state(state,
> -
> plane);
> + struct drm_plane_state *new_state =
> drm_atomic_get_new_plane_state(state,
> +
> plane);
> + struct drm_plane_state *old_state =
> drm_atomic_get_old_plane_state(state,
> +
> plane);
> struct lcdif_drm_private *lcdif = to_lcdif_drm_private(plane->dev);
> struct drm_crtc_state *crtc_state;
> + int ret;
> +
> + /* always okay to disable the plane */
> + if (!new_state->fb)
> + return 0;
>
> crtc_state = drm_atomic_get_new_crtc_state(state,
> &lcdif->crtc);
>
> - return drm_atomic_helper_check_plane_state(plane_state,
> crtc_state,
> - DRM_PLANE_NO_SCALING,
> - DRM_PLANE_NO_SCALING,
> - false, true);
> + ret = drm_atomic_helper_check_plane_state(new_state, crtc_state,
> + DRM_PLANE_NO_SCALING,
> + DRM_PLANE_NO_SCALING,
> + false, true);
> + if (ret)
> + return ret;
> +
> + if (old_state->fb && new_state->fb->format != old_state->fb->format)
> + crtc_state->mode_changed = true;
> +
> + return 0;
> }
>
> static void lcdif_plane_primary_atomic_update(struct drm_plane *plane,
> --
> 2.39.2
