On 3/11/26 5:10 AM, Thomas Zimmermann wrote:
> Refactor the existing simple-display callbacks such that they invoke
> helpers compatible with regular atomic modesetting. Allows for adding
> mipi-dbi drives that do not require simple-display helpers. Provide
> initializer macro for elements of the regular modesetting pipeline.
>
> As the new helpers are DRM functions, add the drm_ prefix. Mipi-dbi
> interfaces currently lack this.
>
No documentation comment for this one?
> +int drm_mipi_dbi_plane_helper_atomic_check(struct drm_plane *plane,
> + struct drm_atomic_state *state)
> +{
> + struct drm_plane_state *new_plane_state =
> drm_atomic_get_new_plane_state(state, plane);
> + struct drm_crtc_state *new_crtc_state;
Should this be intialized to NULL?
> + int ret;
> +
> + if (new_plane_state->crtc)
> + new_crtc_state = drm_atomic_get_new_crtc_state(state,
> new_plane_state->crtc);
> +
> + ret = drm_atomic_helper_check_plane_state(new_plane_state,
> new_crtc_state,
This is passing possibily unintialized new_crtc_state to
drm_atomic_helper_check_plane_state().
> + DRM_PLANE_NO_SCALING,
> + DRM_PLANE_NO_SCALING,
> + false, false);
> + if (ret)
> + return ret;
> + else if (!new_plane_state->visible)
> + return 0;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_mipi_dbi_plane_helper_atomic_check);
> +
No documentation comment for this one?
> +int drm_mipi_dbi_crtc_helper_atomic_check(struct drm_crtc *crtc, struct
> drm_atomic_state *state)
> +{
> + struct drm_crtc_state *crtc_state =
> drm_atomic_get_new_crtc_state(state, crtc);
> + int ret;
> +
> + if (!crtc_state->enable)
> + goto out;
> +
> + ret = drm_atomic_helper_check_crtc_primary_plane(crtc_state);
> + if (ret)
> + return ret;
> +
> +out:
> + return drm_atomic_add_affected_planes(state, crtc);
> +}
> +EXPORT_SYMBOL(drm_mipi_dbi_crtc_helper_atomic_check);
> +
This is public now, so documentation comment?
> -static int mipi_dbi_connector_get_modes(struct drm_connector *connector)
> +int drm_mipi_dbi_connector_helper_get_modes(struct drm_connector *connector)
> {
> struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(connector->dev);
>
> return drm_connector_helper_get_modes_fixed(connector, &dbidev->mode);
> }
> +EXPORT_SYMBOL(drm_mipi_dbi_connector_helper_get_modes);
>
> static const struct drm_connector_helper_funcs mipi_dbi_connector_hfuncs = {
> - .get_modes = mipi_dbi_connector_get_modes,
> + DRM_MIPI_DBI_CONNECTOR_HELPER_FUNCS,
> };
>
> static const struct drm_connector_funcs mipi_dbi_connector_funcs = {
> - .reset = drm_atomic_helper_connector_reset,
> - .fill_modes = drm_helper_probe_single_connector_modes,
> + DRM_MIPI_DBI_CONNECTOR_FUNCS,
> .destroy = drm_connector_cleanup,
> - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> };
>
> static int mipi_dbi_rotate_mode(struct drm_display_mode *mode,
> @@ -540,18 +617,15 @@ static int mipi_dbi_rotate_mode(struct drm_display_mode
> *mode,
> }
>
> static const struct drm_mode_config_helper_funcs
> mipi_dbi_mode_config_helper_funcs = {
> - .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
> + DRM_MIPI_DBI_MODE_CONFIG_HELPER_FUNCS,
> };
>
> static const struct drm_mode_config_funcs mipi_dbi_mode_config_funcs = {
> - .fb_create = drm_gem_fb_create_with_dirty,
> - .atomic_check = drm_atomic_helper_check,
> - .atomic_commit = drm_atomic_helper_commit,
> + DRM_MIPI_DBI_MODE_CONFIG_FUNCS,
> };
>
> static const uint32_t mipi_dbi_formats[] = {
> - DRM_FORMAT_RGB565,
> - DRM_FORMAT_XRGB8888,
> + DRM_MIPI_DBI_PLANE_FORMATS,
> };
Why adding these macros? They are only used once, so it seems like it
just makes the code harder to read (one more place you have to jump to).
Maybe this is common style in drm or something?
>
> /**
> @@ -633,8 +707,7 @@ int mipi_dbi_dev_init_with_formats(struct mipi_dbi_dev
> *dbidev,
> unsigned int rotation, size_t tx_buf_size)
> {
> static const uint64_t modifiers[] = {
> - DRM_FORMAT_MOD_LINEAR,
> - DRM_FORMAT_MOD_INVALID
> + DRM_MIPI_DBI_PLANE_FORMAT_MODIFIERS,
> };
> struct drm_device *drm = &dbidev->drm;
> int ret;
