<div dir="ltr"><div dir="ltr">Hi Thomas,<div>Sorry for leaving so long because of chinese new year<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jan 23, 2020 at 5:13 PM Thomas Zimmermann <<a href="mailto:tzimmermann@suse.de">tzimmermann@suse.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br>
<br>
thank you for replacing for GEM code with the generic CMA<br>
implementation. Once it's needed, we can add specialized memory<br>
management to you driver.<br>
<br>
I've looked throught your submission and think you may want to<br>
reconsider the current design. AFAICT, you have some a driver with<br>
generic code and the individual hardware as sub-devices. That's not<br>
strictly wrong or a roadblocker for merging the driver, but it is likely<br>
to become a problem in long-term maintainence.<br>
<br>
Please read below for some details and let me know what you think.<br></blockquote><div> We have multi display controller(dpu), there will be some differences in the hardware.</div><div>So add KMS to manage multiple dpu, like this:</div><div><div><img src="cid:ii_k6ht5w8a0" alt="1.png" width="562" height="268"><br></div></div><div>dpu0 and dpu1 all maybe have multi ip version, so ip core driver is hardware interface,dpu0 and dpu1 driver call ip core interface.</div><div>Our crtcs maybe have different designs like clock/power/controller register. But crtc plane have the same design.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Am 21.01.20 um 09:50 schrieb Kevin Tang:<br>
> From: Kevin Tang <<a href="mailto:kevin.tang@unisoc.com" target="_blank">kevin.tang@unisoc.com</a>><br>
> <br>
> Adds drm support for the Unisoc's display subsystem.<br>
> <br>
> This is drm device and gem driver. This driver provides support for the<br>
> Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.<br>
> <br>
> Cc: Orson Zhai <<a href="mailto:orsonzhai@gmail.com" target="_blank">orsonzhai@gmail.com</a>><br>
> Cc: Baolin Wang <<a href="mailto:baolin.wang@linaro.org" target="_blank">baolin.wang@linaro.org</a>><br>
> Cc: Chunyan Zhang <<a href="mailto:zhang.lyra@gmail.com" target="_blank">zhang.lyra@gmail.com</a>><br>
> Signed-off-by: Kevin Tang <<a href="mailto:kevin.tang@unisoc.com" target="_blank">kevin.tang@unisoc.com</a>><br>
> ---<br>
>  drivers/gpu/drm/Kconfig           |   2 +<br>
>  drivers/gpu/drm/Makefile          |   1 +<br>
>  drivers/gpu/drm/sprd/Kconfig      |  14 ++<br>
>  drivers/gpu/drm/sprd/Makefile     |   9 +<br>
>  drivers/gpu/drm/sprd/sprd_crtc.c  | 212 ++++++++++++++++++++<br>
>  drivers/gpu/drm/sprd/sprd_crtc.h  |  98 ++++++++++<br>
>  drivers/gpu/drm/sprd/sprd_drm.c   | 305 +++++++++++++++++++++++++++++<br>
>  drivers/gpu/drm/sprd/sprd_drm.h   |  16 ++<br>
>  drivers/gpu/drm/sprd/sprd_plane.c | 393 ++++++++++++++++++++++++++++++++++++++<br>
>  drivers/gpu/drm/sprd/sprd_plane.h |  36 ++++<br>
>  10 files changed, 1086 insertions(+)<br>
>  create mode 100644 drivers/gpu/drm/sprd/Kconfig<br>
>  create mode 100644 drivers/gpu/drm/sprd/Makefile<br>
>  create mode 100644 drivers/gpu/drm/sprd/sprd_crtc.c<br>
>  create mode 100644 drivers/gpu/drm/sprd/sprd_crtc.h<br>
>  create mode 100644 drivers/gpu/drm/sprd/sprd_drm.c<br>
>  create mode 100644 drivers/gpu/drm/sprd/sprd_drm.h<br>
>  create mode 100644 drivers/gpu/drm/sprd/sprd_plane.c<br>
>  create mode 100644 drivers/gpu/drm/sprd/sprd_plane.h<br>
> <br>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig<br>
> index bfdadc3..cead12c 100644<br>
> --- a/drivers/gpu/drm/Kconfig<br>
> +++ b/drivers/gpu/drm/Kconfig<br>
> @@ -387,6 +387,8 @@ source "drivers/gpu/drm/aspeed/Kconfig"<br>
>  <br>
>  source "drivers/gpu/drm/mcde/Kconfig"<br>
>  <br>
> +source "drivers/gpu/drm/sprd/Kconfig"<br>
> +<br>
>  # Keep legacy drivers last<br>
>  <br>
>  menuconfig DRM_LEGACY<br>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile<br>
> index 9f1c7c4..85ca211 100644<br>
> --- a/drivers/gpu/drm/Makefile<br>
> +++ b/drivers/gpu/drm/Makefile<br>
> @@ -122,3 +122,4 @@ obj-$(CONFIG_DRM_LIMA)  += lima/<br>
>  obj-$(CONFIG_DRM_PANFROST) += panfrost/<br>
>  obj-$(CONFIG_DRM_ASPEED_GFX) += aspeed/<br>
>  obj-$(CONFIG_DRM_MCDE) += mcde/<br>
> +obj-$(CONFIG_DRM_SPRD) += sprd/<br>
> diff --git a/drivers/gpu/drm/sprd/Kconfig b/drivers/gpu/drm/sprd/Kconfig<br>
> new file mode 100644<br>
> index 0000000..79f286b<br>
> --- /dev/null<br>
> +++ b/drivers/gpu/drm/sprd/Kconfig<br>
> @@ -0,0 +1,14 @@<br>
> +config DRM_SPRD<br>
> +     tristate "DRM Support for Unisoc SoCs Platform"<br>
> +     depends on ARCH_SPRD<br>
> +     depends on DRM && OF<br>
> +     select DRM_KMS_HELPER<br>
> +     select DRM_GEM_CMA_HELPER<br>
> +     select DRM_KMS_CMA_HELPER<br>
> +     select DRM_MIPI_DSI<br>
> +     select DRM_PANEL<br>
> +     select VIDEOMODE_HELPERS<br>
> +     select BACKLIGHT_CLASS_DEVICE<br>
> +     help<br>
> +       Choose this option if you have a Unisoc chipsets.<br>
> +       If M is selected the module will be called sprd-drm.<br>
> \ No newline at end of file<br>
> diff --git a/drivers/gpu/drm/sprd/Makefile b/drivers/gpu/drm/sprd/Makefile<br>
> new file mode 100644<br>
> index 0000000..8a21c23<br>
> --- /dev/null<br>
> +++ b/drivers/gpu/drm/sprd/Makefile<br>
> @@ -0,0 +1,9 @@<br>
> +# SPDX-License-Identifier: GPL-2.0<br>
> +<br>
> +ccflags-y += -Iinclude/drm<br>
> +<br>
> +subdir-ccflags-y += -I$(src)<br>
> +<br>
> +obj-y := sprd_drm.o \<br>
> +     sprd_crtc.o \<br>
> +     sprd_plane.o<br>
> \ No newline at end of file<br>
> diff --git a/drivers/gpu/drm/sprd/sprd_crtc.c b/drivers/gpu/drm/sprd/sprd_crtc.c<br>
> new file mode 100644<br>
> index 0000000..4a35281<br>
> --- /dev/null<br>
> +++ b/drivers/gpu/drm/sprd/sprd_crtc.c<br>
> @@ -0,0 +1,212 @@<br>
> +// SPDX-License-Identifier: GPL-2.0<br>
> +/*<br>
> + * Copyright (C) 2019 Unisoc Inc.<br>
> + */<br>
> +<br>
> +#include <linux/dma-buf.h><br>
> +<br>
> +#include <drm/drm_atomic_helper.h><br>
> +#include <drm/drm_crtc_helper.h><br>
> +#include <drm/drm_gem_framebuffer_helper.h><br>
> +#include <drm/drm_plane_helper.h><br>
> +#include <drm/drm_vblank.h><br>
> +<br>
> +#include "sprd_drm.h"<br>
> +#include "sprd_crtc.h"<br>
> +<br>
> +static void sprd_crtc_mode_set_nofb(struct drm_crtc *crtc)<br>
> +{<br>
> +     struct sprd_crtc *sprd_crtc = to_sprd_crtc(crtc);<br>
> +<br>
> +     if (sprd_crtc->ops->mode_set_nofb)<br>
> +             return sprd_crtc->ops->mode_set_nofb(sprd_crtc);<br>
> +}<br>
> +<br>
> +static enum drm_mode_status sprd_crtc_mode_valid(struct drm_crtc *crtc,<br>
> +     const struct drm_display_mode *mode)<br>
> +{<br>
> +     struct sprd_crtc *sprd_crtc = to_sprd_crtc(crtc);<br>
> +<br>
> +     if (sprd_crtc->ops->mode_valid)<br>
> +             return sprd_crtc->ops->mode_valid(sprd_crtc, mode);<br>
> +<br>
> +     return MODE_OK;<br>
> +}<br>
> +<br>
> +static int sprd_crtc_atomic_check(struct drm_crtc *crtc,<br>
> +                                  struct drm_crtc_state *state)<br>
> +{<br>
> +     struct sprd_crtc *sprd_crtc = to_sprd_crtc(crtc);<br>
> +<br>
> +     if (!state->enable)<br>
> +             return 0;<br>
> +<br>
> +     if (sprd_crtc->ops->atomic_check)<br>
> +             return sprd_crtc->ops->atomic_check(sprd_crtc, state);<br>
> +<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static void sprd_crtc_atomic_begin(struct drm_crtc *crtc,<br>
> +                                  struct drm_crtc_state *old_crtc_state)<br>
> +{<br>
> +     struct sprd_crtc *sprd_crtc = to_sprd_crtc(crtc);<br>
> +<br>
> +     if (sprd_crtc->ops->atomic_begin)<br>
> +             sprd_crtc->ops->atomic_begin(sprd_crtc);<br>
> +}<br>
> +<br>
> +static void sprd_crtc_atomic_enable(struct drm_crtc *crtc,<br>
> +                                       struct drm_crtc_state *old_state)<br>
> +{<br>
> +     struct sprd_crtc *sprd_crtc = to_sprd_crtc(crtc);<br>
> +<br>
> +     if (sprd_crtc->ops->atomic_enable)<br>
> +             sprd_crtc->ops->atomic_enable(sprd_crtc);<br>
> +<br>
> +     drm_crtc_vblank_on(crtc);<br>
> +}<br>
> +<br>
> +static void sprd_crtc_atomic_disable(struct drm_crtc *crtc,<br>
> +                                        struct drm_crtc_state *old_state)<br>
> +{<br>
> +     struct sprd_crtc *sprd_crtc = to_sprd_crtc(crtc);<br>
> +<br>
> +     drm_crtc_vblank_off(crtc);<br>
> +<br>
> +     if (sprd_crtc->ops->atomic_disable)<br>
> +             sprd_crtc->ops->atomic_disable(sprd_crtc);<br>
> +<br>
> +     if (crtc->state->event && !crtc->state->active) {<br>
> +             spin_lock_irq(&crtc->dev->event_lock);<br>
> +             drm_crtc_send_vblank_event(crtc, crtc->state->event);<br>
> +             spin_unlock_irq(&crtc->dev->event_lock);<br>
> +<br>
> +             crtc->state->event = NULL;<br>
> +     }<br>
> +}<br>
> +<br>
> +static void sprd_crtc_atomic_flush(struct drm_crtc *crtc,<br>
> +                                  struct drm_crtc_state *old_crtc_state)<br>
> +{<br>
> +     struct sprd_crtc *sprd_crtc = to_sprd_crtc(crtc);<br>
> +<br>
> +     if (sprd_crtc->ops->atomic_flush)<br>
> +             sprd_crtc->ops->atomic_flush(sprd_crtc);<br>
> +<br>
> +     spin_lock_irq(&crtc->dev->event_lock);<br>
> +     if (crtc->state->event) {<br>
> +             drm_crtc_send_vblank_event(crtc, crtc->state->event);<br>
> +             crtc->state->event = NULL;<br>
> +     }<br>
> +     spin_unlock_irq(&crtc->dev->event_lock);<br>
> +}<br>
> +<br>
> +static const struct drm_crtc_helper_funcs sprd_crtc_helper_funcs = {<br>
> +     .mode_set_nofb  = sprd_crtc_mode_set_nofb,<br>
> +     .mode_valid     = sprd_crtc_mode_valid,<br>
> +     .atomic_check   = sprd_crtc_atomic_check,<br>
> +     .atomic_begin   = sprd_crtc_atomic_begin,<br>
> +     .atomic_enable  = sprd_crtc_atomic_enable,<br>
> +     .atomic_disable = sprd_crtc_atomic_disable,<br>
> +     .atomic_flush   = sprd_crtc_atomic_flush,<br>
> +};<br>
<br>
This is just boilerplate resulting from the current design. Instead of<br>
having struct sprd_crtc_ops and these mid-layer functions, simply give<br>
each hardware device its own instance of drm_crtc_helper_funcs. If<br>
hardware can share some of the implementation, put it into a common<br>
helper function.<br></blockquote><div>The current design, I refer to exynos design. i think it's also a good idea.</div><div>"It is likely to become a problem in long-term maintainence", how to understand it?<br></div><div><br></div></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +<br>
> +static void sprd_crtc_cleanup(struct drm_crtc *crtc)<br>
> +{<br>
> +     struct sprd_crtc *sprd_crtc = to_sprd_crtc(crtc);<br>
> +<br>
> +     drm_crtc_cleanup(crtc);<br>
> +     kfree(sprd_crtc);<br>
> +}<br>
> +<br>
> +static int sprd_crtc_enable_vblank(struct drm_crtc *crtc)<br>
> +{<br>
> +     struct sprd_crtc *sprd_crtc = to_sprd_crtc(crtc);<br>
> +<br>
> +     if (sprd_crtc->ops->enable_vblank)<br>
> +             return sprd_crtc->ops->enable_vblank(sprd_crtc);<br>
> +<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static void sprd_crtc_disable_vblank(struct drm_crtc *crtc)<br>
> +{<br>
> +     struct sprd_crtc *sprd_crtc = to_sprd_crtc(crtc);<br>
> +<br>
> +     if (sprd_crtc->ops->disable_vblank)<br>
> +             sprd_crtc->ops->disable_vblank(sprd_crtc);<br>
> +}<br>
> +<br>
> +static const struct drm_crtc_funcs sprd_crtc_funcs = {<br>
> +     .destroy        = sprd_crtc_cleanup,<br>
> +     .set_config     = drm_atomic_helper_set_config,<br>
> +     .page_flip      = drm_atomic_helper_page_flip,<br>
> +     .reset          = drm_atomic_helper_crtc_reset,<br>
> +     .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,<br>
> +     .atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,<br>
> +     .enable_vblank  = sprd_crtc_enable_vblank,<br>
> +     .disable_vblank = sprd_crtc_disable_vblank,<br>
> +};<br>
<br>
Same as with the CRTC helper functions.<br>
<br>
> +<br>
> +struct sprd_crtc *sprd_crtc_init(struct drm_device *drm,<br>
> +                                     struct drm_plane *plane,<br>
> +                                     enum sprd_crtc_output_type type,<br>
> +                                     const struct sprd_crtc_ops *ops,<br>
<br>
Rather hand-in the hardware's crtc and crtc-helper instances.<br>
<br>
> +                                     void *dpu)<br>
> +{<br>
> +     struct sprd_crtc *sprd_crtc;<br>
> +     int ret;<br>
> +<br>
> +     sprd_crtc = kzalloc(sizeof(*sprd_crtc), GFP_KERNEL);<br>
> +     if (!sprd_crtc)<br>
> +             return ERR_PTR(-ENOMEM);<br>
> +<br>
> +     sprd_crtc->type = type;<br>
> +     sprd_crtc->ops = ops;<br>
> +     sprd_crtc->dpu = dpu;<br>
> +<br>
> +     ret = drm_crtc_init_with_planes(drm, &sprd_crtc->base, plane, NULL,<br>
> +                                     &sprd_crtc_funcs, NULL);<br>
> +     if (ret < 0) {<br>
> +             DRM_ERROR("failed to init crtc.\n");<br>
> +             goto err_crtc;<br>
> +     }<br>
> +<br>
> +     drm_mode_crtc_set_gamma_size(&sprd_crtc->base, 256);<br>
> +<br>
> +     drm_crtc_helper_add(&sprd_crtc->base, &sprd_crtc_helper_funcs);<br>
> +<br>
> +     return sprd_crtc;<br>
> +<br>
> +err_crtc:<br>
> +     plane->funcs->destroy(plane);<br>
> +     kfree(sprd_crtc);<br>
> +     return ERR_PTR(ret);<br>
> +}<br>
> +<br>
> +struct sprd_crtc *sprd_crtc_get_by_type(struct drm_device *drm,<br>
> +                                    enum sprd_crtc_output_type out_type)<br>
> +{<br>
> +     struct drm_crtc *crtc;<br>
> +<br>
> +     drm_for_each_crtc(crtc, drm)<br>
> +             if (to_sprd_crtc(crtc)->type == out_type)<br>
> +                     return to_sprd_crtc(crtc);<br>
> +<br>
> +     return ERR_PTR(-EPERM);<br>
> +}<br>
> +<br>
> +int sprd_drm_set_possible_crtcs(struct drm_encoder *encoder,<br>
> +             enum sprd_crtc_output_type out_type)<br>
> +{<br>
> +     struct sprd_crtc *crtc = sprd_crtc_get_by_type(encoder->dev,<br>
> +                                             out_type);<br>
> +<br>
> +     if (IS_ERR(crtc))<br>
> +             return PTR_ERR(crtc);<br>
> +<br>
> +     encoder->possible_crtcs = drm_crtc_mask(&crtc->base);<br>
> +<br>
> +     return 0;<br>
> +}<br>
> diff --git a/drivers/gpu/drm/sprd/sprd_crtc.h b/drivers/gpu/drm/sprd/sprd_crtc.h<br>
> new file mode 100644<br>
> index 0000000..9dbb2eb<br>
> --- /dev/null<br>
> +++ b/drivers/gpu/drm/sprd/sprd_crtc.h<br>
> @@ -0,0 +1,98 @@<br>
> +/* SPDX-License-Identifier: GPL-2.0 */<br>
> +/*<br>
> + * Copyright (C) 2019 Unisoc Inc.<br>
> + */<br>
> +<br>
> +#ifndef __SPRD_CRTC_H__<br>
> +#define __SPRD_CRTC_H__<br>
> +<br>
> +#include <drm/drm_crtc.h><br>
> +#include <drm/drm_encoder.h><br>
> +#include <drm/drm_fourcc.h><br>
> +#include <drm/drm_plane.h><br>
> +#include <drm/drm_print.h><br>
> +<br>
> +#define to_sprd_crtc(x)                      container_of(x, struct sprd_crtc, base)<br>
> +<br>
> +enum sprd_crtc_output_type {<br>
> +     SPRD_DISPLAY_TYPE_NONE,<br>
> +     /* RGB or CPU Interface */<br>
> +     SPRD_DISPLAY_TYPE_LCD,<br>
> +     /* DisplayPort Interface */<br>
> +     SPRD_DISPLAY_TYPE_DP,<br>
> +     /* HDMI Interface */<br>
> +     SPRD_DISPLAY_TYPE_HDMI,<br>
> +};<br>
> +<br>
> +struct dpu_capability {<br>
> +     u32 max_layers;<br>
> +     const u32 *fmts_ptr;<br>
> +     u32 fmts_cnt;<br>
> +};<br>
> +<br>
> +struct sprd_dpu_layer {<br>
> +     u8 index;<br>
> +     u8 planes;<br>
> +     u32 addr[4];<br>
> +     u32 pitch[4];<br>
> +     s16 src_x;<br>
> +     s16 src_y;<br>
> +     s16 src_w;<br>
> +     s16 src_h;<br>
> +     s16 dst_x;<br>
> +     s16 dst_y;<br>
> +     u16 dst_w;<br>
> +     u16 dst_h;<br>
> +     u32 format;<br>
> +     u32 alpha;<br>
> +     u32 blending;<br>
> +     u32 rotation;<br>
> +     u32 xfbc;<br>
> +     u32 height;<br>
> +     u32 header_size_r;<br>
> +     u32 header_size_y;<br>
> +     u32 header_size_uv;<br>
> +     u32 y2r_coef;<br>
> +     u8 pallete_en;<br>
> +     u32 pallete_color;<br>
> +};<br>
> +<br>
> +struct sprd_crtc {<br>
> +     struct drm_crtc base;<br>
> +     enum sprd_crtc_output_type type;<br>
> +     const struct sprd_crtc_ops *ops;<br>
> +     struct sprd_dpu_layer *layers;<br>
> +     u8 pending_planes;<br>
> +     void *dpu;<br>
> +};<br>
> +<br>
> +struct sprd_crtc_ops {<br>
> +     void (*atomic_enable)(struct sprd_crtc *crtc);<br>
> +     void (*atomic_disable)(struct sprd_crtc *crtc);<br>
> +     int (*enable_vblank)(struct sprd_crtc *crtc);<br>
> +     void (*disable_vblank)(struct sprd_crtc *crtc);<br>
> +     enum drm_mode_status (*mode_valid)(struct sprd_crtc *crtc,<br>
> +             const struct drm_display_mode *mode);<br>
> +     void (*mode_set_nofb)(struct sprd_crtc *crtc);<br>
> +     int (*atomic_check)(struct sprd_crtc *crtc,<br>
> +                         struct drm_crtc_state *state);<br>
> +     void (*atomic_begin)(struct sprd_crtc *crtc);<br>
> +     void (*atomic_flush)(struct sprd_crtc *crtc);<br>
> +<br>
> +     int (*prepare_fb)(struct sprd_crtc *crtc,<br>
> +                       struct drm_plane_state *new_state);<br>
> +     void (*cleanup_fb)(struct sprd_crtc *crtc,<br>
> +                        struct drm_plane_state *old_state);<br>
> +     void (*atomic_update)(struct sprd_crtc *crtc,<br>
> +                          struct drm_plane *plane);<br>
> +};<br>
> +<br>
> +struct sprd_crtc *sprd_crtc_init(struct drm_device *drm,<br>
> +                                     struct drm_plane *plane,<br>
> +                                     enum sprd_crtc_output_type type,<br>
> +                                     const struct sprd_crtc_ops *ops,<br>
> +                                     void *dpu);<br>
> +int sprd_drm_set_possible_crtcs(struct drm_encoder *encoder,<br>
> +             enum sprd_crtc_output_type out_type);<br>
> +<br>
> +#endif<br>
> diff --git a/drivers/gpu/drm/sprd/sprd_drm.c b/drivers/gpu/drm/sprd/sprd_drm.c<br>
> new file mode 100644<br>
> index 0000000..d63b7e3<br>
> --- /dev/null<br>
> +++ b/drivers/gpu/drm/sprd/sprd_drm.c<br>
> @@ -0,0 +1,305 @@<br>
> +// SPDX-License-Identifier: GPL-2.0<br>
> +/*<br>
> + * Copyright (C) 2019 Unisoc Inc.<br>
> + */<br>
> +<br>
> +#include <linux/component.h><br>
> +#include <linux/dma-mapping.h><br>
> +#include <linux/module.h><br>
> +#include <linux/mutex.h><br>
> +#include <linux/of_graph.h><br>
> +#include <linux/of_platform.h><br>
> +<br>
> +#include <drm/drm_atomic_helper.h><br>
> +#include <drm/drm_crtc_helper.h><br>
> +#include <drm/drm_drv.h><br>
> +#include <drm/drm_gem_cma_helper.h><br>
> +#include <drm/drm_gem_framebuffer_helper.h><br>
> +#include <drm/drm_probe_helper.h><br>
> +#include <drm/drm_vblank.h><br>
> +<br>
> +#include "sprd_drm.h"<br>
> +<br>
> +#define DRIVER_NAME  "sprd"<br>
> +#define DRIVER_DESC  "Spreadtrum SoCs' DRM Driver"<br>
> +#define DRIVER_DATE  "20191101"<br>
> +#define DRIVER_MAJOR 1<br>
> +#define DRIVER_MINOR 0<br>
> +<br>
> +static const struct drm_mode_config_helper_funcs sprd_drm_mode_config_helper = {<br>
> +     .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,<br>
> +};<br>
> +<br>
> +static const struct drm_mode_config_funcs sprd_drm_mode_config_funcs = {<br>
> +     .fb_create = drm_gem_fb_create,<br>
> +     .atomic_check = drm_atomic_helper_check,<br>
> +     .atomic_commit = drm_atomic_helper_commit,<br>
> +};<br>
> +<br>
> +static void sprd_drm_mode_config_init(struct drm_device *drm)<br>
> +{<br>
> +     drm_mode_config_init(drm);<br>
> +<br>
> +     drm->mode_config.min_width = 0;<br>
> +     drm->mode_config.min_height = 0;<br>
> +     drm->mode_config.max_width = 8192;<br>
> +     drm->mode_config.max_height = 8192;<br>
> +     drm->mode_config.allow_fb_modifiers = true;<br>
> +<br>
> +     drm->mode_config.funcs = &sprd_drm_mode_config_funcs;<br>
> +     drm->mode_config.helper_private = &sprd_drm_mode_config_helper;<br>
> +}<br>
> +<br>
> +static const struct file_operations sprd_drm_fops = {<br>
> +     .owner          = THIS_MODULE,<br>
> +     .open           = drm_open,<br>
> +     .release        = drm_release,<br>
> +     .unlocked_ioctl = drm_ioctl,<br>
> +     .compat_ioctl   = drm_compat_ioctl,<br>
> +     .poll           = drm_poll,<br>
> +     .read           = drm_read,<br>
> +     .llseek         = no_llseek,<br>
> +     .mmap           = drm_gem_cma_mmap,<br>
> +};<br>
> +<br>
> +static struct drm_driver sprd_drm_drv = {<br>
> +     .driver_features        = DRIVER_GEM | DRIVER_ATOMIC,<br>
> +     .fops                   = &sprd_drm_fops,<br>
> +     .gem_vm_ops             = &drm_gem_cma_vm_ops,<br>
> +     .gem_free_object_unlocked       = drm_gem_cma_free_object,<br>
> +     .dumb_create            = drm_gem_cma_dumb_create,<br>
> +     .prime_fd_to_handle     = drm_gem_prime_fd_to_handle,<br>
> +     .gem_prime_import       = drm_gem_prime_import,<br>
> +     .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,<br>
<br>
Rather set these pointers with DRM_GEM_CMA_VMAP_DRIVER_OPS.<br>
<br>
> +<br>
> +     .name                   = DRIVER_NAME,<br>
> +     .desc                   = DRIVER_DESC,<br>
> +     .date                   = DRIVER_DATE,<br>
> +     .major                  = DRIVER_MAJOR,<br>
> +     .minor                  = DRIVER_MINOR,<br>
> +};<br>
> +<br>
> +static int sprd_drm_bind(struct device *dev)<br>
> +{<br>
> +     struct drm_device *drm;<br>
> +     struct sprd_drm *sprd;<br>
> +     int err;<br>
> +<br>
> +     drm = drm_dev_alloc(&sprd_drm_drv, dev);<br>
> +     if (IS_ERR(drm))<br>
> +             return PTR_ERR(drm);<br>
> +<br>
> +     dev_set_drvdata(dev, drm);<br>
> +<br>
> +     sprd = devm_kzalloc(drm->dev, sizeof(*sprd), GFP_KERNEL);<br>
> +     if (!sprd) {<br>
> +             err = -ENOMEM;<br>
> +             goto err_free_drm;<br>
> +     }<br>
> +     drm->dev_private = sprd;<br>
> +<br>
> +     sprd_drm_mode_config_init(drm);<br>
> +<br>
> +     /* bind and init sub drivers */<br>
> +     err = component_bind_all(drm->dev, drm);<br>
> +     if (err) {<br>
> +             DRM_ERROR("failed to bind all component.\n");<br>
> +             goto err_dc_cleanup;<br>
> +     }<br>
> +<br>
> +     /* vblank init */<br>
> +     err = drm_vblank_init(drm, drm->mode_config.num_crtc);<br>
> +     if (err) {<br>
> +             DRM_ERROR("failed to initialize vblank.\n");<br>
> +             goto err_unbind_all;<br>
> +     }<br>
> +     /* with irq_enabled = true, we can use the vblank feature. */<br>
> +     drm->irq_enabled = true;<br>
<br>
This line indicates the problem's design. The irq is initialized in the<br>
sub-device code, but the device state is set here. Instead both should<br>
be set in the same place.<br></blockquote><div>Everyone seems to use it like this, so why can't follow others?</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +<br>
> +     /* reset all the states of crtc/plane/encoder/connector */<br>
> +     drm_mode_config_reset(drm);<br>
> +<br>
> +     /* init kms poll for handling hpd */<br>
> +     drm_kms_helper_poll_init(drm);<br>
<br>
Most of this function's code should be moved into the sub-device bind<br>
function.<br>
<br>
Here, maybe do:<br>
<br>
 * allocate device structures<br>
 * call component_bind_all()<br>
 * on success, register device<br>
<br>
The sub-device function should then do<br>
<br>
 * init modesetting, crtc, planes, etc.<br>
 * do drm_mode_config_reset()<br>
 * init vblanking<br>
 * init the irq<br>
 * do drm_kms_helper_poll_init()<br>
<br>
roughtly in that order. It makes sense to call drm_vblank_init() after<br>
drm_mode_config_reset(), as vblanking uses some of the mode-config fields.<br>
<br>
> +<br>
> +     err = drm_dev_register(drm, 0);<br>
> +     if (err < 0)<br>
> +             goto err_kms_helper_poll_fini;<br>
<br>
You don't need/want an fbdev console?<br></blockquote><div>Yes, fbdev is not supported, maybe support later for LoT(?)</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Best regards<br>
Thomas<br>
<br>
> +<br>
> +     return 0;<br>
> +<br>
> +err_kms_helper_poll_fini:<br>
> +     drm_kms_helper_poll_fini(drm);<br>
> +err_unbind_all:<br>
> +     component_unbind_all(drm->dev, drm);<br>
> +err_dc_cleanup:<br>
> +     drm_mode_config_cleanup(drm);<br>
> +err_free_drm:<br>
> +     drm_dev_put(drm);<br>
> +     return err;<br>
> +}<br>
> +<br>
> +static void sprd_drm_unbind(struct device *dev)<br>
> +{<br>
> +     drm_put_dev(dev_get_drvdata(dev));<br>
> +}<br>
> +<br>
> +static const struct component_master_ops drm_component_ops = {<br>
> +     .bind = sprd_drm_bind,<br>
> +     .unbind = sprd_drm_unbind,<br>
> +};<br>
> +<br>
> +static int compare_of(struct device *dev, void *data)<br>
> +{<br>
> +     struct device_node *np = data;<br>
> +<br>
> +     DRM_DEBUG("compare %s\n", np->full_name);<br>
> +<br>
> +     return dev->of_node == np;<br>
> +}<br>
> +<br>
> +static int sprd_drm_component_probe(struct device *dev,<br>
> +                        const struct component_master_ops *m_ops)<br>
> +{<br>
> +     struct device_node *ep, *port, *remote;<br>
> +     struct component_match *match = NULL;<br>
> +     int i;<br>
> +<br>
> +     if (!dev->of_node)<br>
> +             return -EINVAL;<br>
> +<br>
> +     /*<br>
> +      * Bind the crtc's ports first, so that drm_of_find_possible_crtcs()<br>
> +      * called from encoder's .bind callbacks works as expected<br>
> +      */<br>
> +     for (i = 0; ; i++) {<br>
> +             port = of_parse_phandle(dev->of_node, "ports", i);<br>
> +             if (!port)<br>
> +                     break;<br>
> +<br>
> +             if (!of_device_is_available(port->parent)) {<br>
> +                     of_node_put(port);<br>
> +                     continue;<br>
> +             }<br>
> +<br>
> +             component_match_add(dev, &match, compare_of, port->parent);<br>
> +             of_node_put(port);<br>
> +     }<br>
> +<br>
> +     if (i == 0) {<br>
> +             dev_err(dev, "missing 'ports' property\n");<br>
> +             return -ENODEV;<br>
> +     }<br>
> +<br>
> +     if (!match) {<br>
> +             dev_err(dev, "no available port\n");<br>
> +             return -ENODEV;<br>
> +     }<br>
> +<br>
> +     /*<br>
> +      * For bound crtcs, bind the encoders attached to their remote endpoint<br>
> +      */<br>
> +     for (i = 0; ; i++) {<br>
> +             port = of_parse_phandle(dev->of_node, "ports", i);<br>
> +             if (!port)<br>
> +                     break;<br>
> +<br>
> +             if (!of_device_is_available(port->parent)) {<br>
> +                     of_node_put(port);<br>
> +                     continue;<br>
> +             }<br>
> +<br>
> +             for_each_child_of_node(port, ep) {<br>
> +                     remote = of_graph_get_remote_port_parent(ep);<br>
> +                     if (!remote || !of_device_is_available(remote)) {<br>
> +                             of_node_put(remote);<br>
> +                             continue;<br>
> +                     } else if (!of_device_is_available(remote->parent)) {<br>
> +                             dev_warn(dev, "parent device of %s is not available\n",<br>
> +                                      remote->full_name);<br>
> +                             of_node_put(remote);<br>
> +                             continue;<br>
> +                     }<br>
> +<br>
> +                     component_match_add(dev, &match, compare_of, remote);<br>
> +                     of_node_put(remote);<br>
> +             }<br>
> +             of_node_put(port);<br>
> +     }<br>
> +<br>
> +     return component_master_add_with_match(dev, m_ops, match);<br>
> +}<br>
> +<br>
> +static int sprd_drm_probe(struct platform_device *pdev)<br>
> +{<br>
> +     int ret;<br>
> +<br>
> +     ret = dma_set_mask_and_coherent(&pdev->dev, ~0);<br>
> +     if (ret) {<br>
> +             DRM_ERROR("dma_set_mask_and_coherent failed (%d)\n", ret);<br>
> +             return ret;<br>
> +     }<br>
> +<br>
> +     return sprd_drm_component_probe(&pdev->dev, &drm_component_ops);<br>
> +}<br>
> +<br>
> +static int sprd_drm_remove(struct platform_device *pdev)<br>
> +{<br>
> +     component_master_del(&pdev->dev, &drm_component_ops);<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static void sprd_drm_shutdown(struct platform_device *pdev)<br>
> +{<br>
> +     struct drm_device *drm = platform_get_drvdata(pdev);<br>
> +<br>
> +     if (!drm) {<br>
> +             DRM_WARN("drm device is not available, no shutdown\n");<br>
> +             return;<br>
> +     }<br>
> +<br>
> +     drm_atomic_helper_shutdown(drm);<br>
> +}<br>
> +<br>
> +static const struct of_device_id drm_match_table[] = {<br>
> +     { .compatible = "sprd,display-subsystem", },<br>
> +     {},<br>
> +};<br>
> +MODULE_DEVICE_TABLE(of, drm_match_table);<br>
> +<br>
> +static struct platform_driver sprd_drm_driver = {<br>
> +     .probe = sprd_drm_probe,<br>
> +     .remove = sprd_drm_remove,<br>
> +     .shutdown = sprd_drm_shutdown,<br>
> +     .driver = {<br>
> +             .name = "sprd-drm-drv",<br>
> +             .of_match_table = drm_match_table,<br>
> +     },<br>
> +};<br>
> +<br>
> +static struct platform_driver *sprd_drm_drivers[]  = {<br>
> +     &sprd_drm_driver,<br>
> +};<br>
> +<br>
> +static int __init sprd_drm_init(void)<br>
> +{<br>
> +     int ret;<br>
> +<br>
> +     ret = platform_register_drivers(sprd_drm_drivers,<br>
> +                                     ARRAY_SIZE(sprd_drm_drivers));<br>
> +     return ret;<br>
> +}<br>
> +<br>
> +static void __exit sprd_drm_exit(void)<br>
> +{<br>
> +     platform_unregister_drivers(sprd_drm_drivers,<br>
> +                                 ARRAY_SIZE(sprd_drm_drivers));<br>
> +}<br>
> +<br>
> +module_init(sprd_drm_init);<br>
> +module_exit(sprd_drm_exit);<br>
> +<br>
> +MODULE_AUTHOR("Leon He <<a href="mailto:leon.he@unisoc.com" target="_blank">leon.he@unisoc.com</a>>");<br>
> +MODULE_AUTHOR("Kevin Tang <<a href="mailto:kevin.tang@unisoc.com" target="_blank">kevin.tang@unisoc.com</a>>");<br>
> +MODULE_DESCRIPTION("Unisoc DRM KMS Master Driver");<br>
> +MODULE_LICENSE("GPL v2");<br>
> diff --git a/drivers/gpu/drm/sprd/sprd_drm.h b/drivers/gpu/drm/sprd/sprd_drm.h<br>
> new file mode 100644<br>
> index 0000000..137cb27<br>
> --- /dev/null<br>
> +++ b/drivers/gpu/drm/sprd/sprd_drm.h<br>
> @@ -0,0 +1,16 @@<br>
> +/* SPDX-License-Identifier: GPL-2.0 */<br>
> +/*<br>
> + * Copyright (C) 2019 Unisoc Inc.<br>
> + */<br>
> +<br>
> +#ifndef _SPRD_DRM_H_<br>
> +#define _SPRD_DRM_H_<br>
> +<br>
> +#include <drm/drm_atomic.h><br>
> +#include <drm/drm_print.h><br>
> +<br>
> +struct sprd_drm {<br>
> +     struct drm_device *drm;<br>
> +};<br>
> +<br>
> +#endif /* _SPRD_DRM_H_ */<br>
> diff --git a/drivers/gpu/drm/sprd/sprd_plane.c b/drivers/gpu/drm/sprd/sprd_plane.c<br>
> new file mode 100644<br>
> index 0000000..15ebb97<br>
> --- /dev/null<br>
> +++ b/drivers/gpu/drm/sprd/sprd_plane.c<br>
> @@ -0,0 +1,393 @@<br>
> +// SPDX-License-Identifier: GPL-2.0<br>
> +/*<br>
> + * Copyright (C) 2019 Unisoc Inc.<br>
> + */<br>
> +<br>
> +#include <linux/dma-buf.h><br>
> +<br>
> +#include <drm/drm_atomic_helper.h><br>
> +#include <drm/drm_fb_cma_helper.h><br>
> +#include <drm/drm_crtc_helper.h><br>
> +#include <drm/drm_fourcc.h><br>
> +#include <drm/drm_gem_cma_helper.h><br>
> +#include <drm/drm_gem_framebuffer_helper.h><br>
> +#include <drm/drm_plane_helper.h><br>
> +<br>
> +#include "sprd_crtc.h"<br>
> +#include "sprd_drm.h"<br>
> +#include "sprd_plane.h"<br>
> +<br>
> +static int sprd_plane_prepare_fb(struct drm_plane *plane,<br>
> +                             struct drm_plane_state *new_state)<br>
> +{<br>
> +     struct drm_plane_state *curr_state = plane->state;<br>
> +     struct sprd_crtc *crtc = to_sprd_crtc(new_state->crtc);<br>
> +<br>
> +     if ((curr_state->fb == new_state->fb) || !new_state->fb)<br>
> +             return 0;<br>
> +<br>
> +     if (crtc->ops->prepare_fb)<br>
> +             crtc->ops->prepare_fb(crtc, new_state);<br>
> +<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static void sprd_plane_cleanup_fb(struct drm_plane *plane,<br>
> +                             struct drm_plane_state *old_state)<br>
> +{<br>
> +     struct drm_plane_state *curr_state = plane->state;<br>
> +     struct sprd_crtc *crtc = to_sprd_crtc(old_state->crtc);<br>
> +<br>
> +     if ((curr_state->fb == old_state->fb) || !old_state->fb)<br>
> +             return;<br>
> +<br>
> +     if (crtc->ops->cleanup_fb)<br>
> +             crtc->ops->cleanup_fb(crtc, old_state);<br>
> +}<br>
> +<br>
> +static void sprd_plane_atomic_update(struct drm_plane *plane,<br>
> +                                 struct drm_plane_state *old_state)<br>
> +{<br>
> +     struct drm_plane_state *state = plane->state;<br>
> +     struct drm_framebuffer *fb = plane->state->fb;<br>
> +     struct drm_gem_cma_object *cma_obj;<br>
> +     struct sprd_crtc *crtc = to_sprd_crtc(plane->state->crtc);<br>
> +     struct sprd_plane *p = to_sprd_plane(plane);<br>
> +     struct sprd_plane_state *s = to_sprd_plane_state(state);<br>
> +     struct sprd_dpu_layer *layer = &crtc->layers[p->index];<br>
> +     int i;<br>
> +<br>
> +     if (plane->state->crtc->state->active_changed) {<br>
> +             DRM_DEBUG("resume or suspend, no need to update plane\n");<br>
> +             return;<br>
> +     }<br>
> +<br>
> +     if (s->pallete_en) {<br>
> +             layer->index = p->index;<br>
> +             layer->dst_x = state->crtc_x;<br>
> +             layer->dst_y = state->crtc_y;<br>
> +             layer->dst_w = state->crtc_w;<br>
> +             layer->dst_h = state->crtc_h;<br>
> +             layer->alpha = state->alpha;<br>
> +             layer->blending = state->pixel_blend_mode;<br>
> +             layer->pallete_en = s->pallete_en;<br>
> +             layer->pallete_color = s->pallete_color;<br>
> +             crtc->pending_planes++;<br>
> +             DRM_DEBUG("%s() pallete_color = %u, index = %u\n",<br>
> +                     __func__, layer->pallete_color, layer->index);<br>
> +             return;<br>
> +     }<br>
> +<br>
> +     layer->index = p->index;<br>
> +     layer->src_x = state->src_x >> 16;<br>
> +     layer->src_y = state->src_y >> 16;<br>
> +     layer->src_w = state->src_w >> 16;<br>
> +     layer->src_h = state->src_h >> 16;<br>
> +     layer->dst_x = state->crtc_x;<br>
> +     layer->dst_y = state->crtc_y;<br>
> +     layer->dst_w = state->crtc_w;<br>
> +     layer->dst_h = state->crtc_h;<br>
> +     layer->rotation = state->rotation;<br>
> +     layer->blending = state->pixel_blend_mode;<br>
> +     layer->rotation = state->rotation;<br>
> +     layer->planes = fb->format->num_planes;<br>
> +     layer->format = fb->format->format;<br>
> +     layer->xfbc = fb->modifier;<br>
> +     layer->header_size_r = s->fbc_hsize_r;<br>
> +     layer->header_size_y = s->fbc_hsize_y;<br>
> +     layer->header_size_uv = s->fbc_hsize_uv;<br>
> +     layer->y2r_coef = s->y2r_coef;<br>
> +     layer->pallete_en = s->pallete_en;<br>
> +     layer->pallete_color = s->pallete_color;<br>
> +<br>
> +     DRM_DEBUG("%s() alpha = %u, blending = %u, rotation = %u, y2r_coef = %u\n",<br>
> +               __func__, layer->alpha, layer->blending, layer->rotation, s->y2r_coef);<br>
> +<br>
> +     DRM_DEBUG("%s() xfbc = %u, hsize_r = %u, hsize_y = %u, hsize_uv = %u\n",<br>
> +               __func__, layer->xfbc, layer->header_size_r,<br>
> +               layer->header_size_y, layer->header_size_uv);<br>
> +<br>
> +     for (i = 0; i < layer->planes; i++) {<br>
> +             cma_obj = drm_fb_cma_get_gem_obj(fb, i);<br>
> +             layer->addr[i] = cma_obj->paddr + fb->offsets[i];<br>
> +             layer->pitch[i] = fb->pitches[i];<br>
> +     }<br>
> +<br>
> +     crtc->pending_planes++;<br>
> +}<br>
> +<br>
> +static int sprd_plane_atomic_check(struct drm_plane *plane,<br>
> +                               struct drm_plane_state *state)<br>
> +{<br>
> +     DRM_DEBUG("%s()\n", __func__);<br>
> +<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static void sprd_plane_atomic_disable(struct drm_plane *plane,<br>
> +                                  struct drm_plane_state *old_state)<br>
> +{<br>
> +     struct sprd_plane *p = to_sprd_plane(plane);<br>
> +<br>
> +     /*<br>
> +      * NOTE:<br>
> +      * The dpu->core->flip() will disable all the planes each time.<br>
> +      * So there is no need to impliment the atomic_disable() function.<br>
> +      * But this function can not be removed, because it will change<br>
> +      * to call atomic_update() callback instead. Which will cause<br>
> +      * kernel panic in sprd_plane_atomic_update().<br>
> +      *<br>
> +      * We do nothing here but just print a debug log.<br>
> +      */<br>
> +     DRM_DEBUG("%s() layer_id = %u\n", __func__, p->index);<br>
> +}<br>
> +<br>
> +static const struct drm_plane_helper_funcs sprd_plane_helper_funcs = {<br>
> +     .prepare_fb = sprd_plane_prepare_fb,<br>
> +     .cleanup_fb = sprd_plane_cleanup_fb,<br>
> +     .atomic_check = sprd_plane_atomic_check,<br>
> +     .atomic_update = sprd_plane_atomic_update,<br>
> +     .atomic_disable = sprd_plane_atomic_disable,<br>
> +};<br>
> +<br>
> +static void sprd_plane_reset(struct drm_plane *plane)<br>
> +{<br>
> +     struct sprd_plane *p = to_sprd_plane(plane);<br>
> +     struct sprd_plane_state *s;<br>
> +<br>
> +     DRM_INFO("%s()\n", __func__);<br>
> +<br>
> +     if (plane->state) {<br>
> +             __drm_atomic_helper_plane_destroy_state(plane->state);<br>
> +<br>
> +             s = to_sprd_plane_state(plane->state);<br>
> +             memset(s, 0, sizeof(*s));<br>
> +     } else {<br>
> +             s = kzalloc(sizeof(*s), GFP_KERNEL);<br>
> +             if (!s)<br>
> +                     return;<br>
> +             plane->state = &s->state;<br>
> +     }<br>
> +<br>
> +     s->state.plane = plane;<br>
> +     s->state.zpos = p->index;<br>
> +     s->state.alpha = 255;<br>
> +     s->state.pixel_blend_mode = DRM_MODE_BLEND_PIXEL_NONE;<br>
> +}<br>
> +<br>
> +static struct drm_plane_state *<br>
> +sprd_plane_atomic_duplicate_state(struct drm_plane *plane)<br>
> +{<br>
> +     struct sprd_plane_state *s;<br>
> +     struct sprd_plane_state *old_state = to_sprd_plane_state(plane->state);<br>
> +<br>
> +     DRM_DEBUG("%s()\n", __func__);<br>
> +<br>
> +     s = kzalloc(sizeof(*s), GFP_KERNEL);<br>
> +     if (!s)<br>
> +             return NULL;<br>
> +<br>
> +     __drm_atomic_helper_plane_duplicate_state(plane, &s->state);<br>
> +<br>
> +     WARN_ON(s->state.plane != plane);<br>
> +<br>
> +     s->fbc_hsize_r = old_state->fbc_hsize_r;<br>
> +     s->fbc_hsize_y = old_state->fbc_hsize_y;<br>
> +     s->fbc_hsize_uv = old_state->fbc_hsize_uv;<br>
> +     s->y2r_coef = old_state->y2r_coef;<br>
> +     s->pallete_en = old_state->pallete_en;<br>
> +     s->pallete_color = old_state->pallete_color;<br>
> +<br>
> +     return &s->state;<br>
> +}<br>
> +<br>
> +static void sprd_plane_atomic_destroy_state(struct drm_plane *plane,<br>
> +                                         struct drm_plane_state *state)<br>
> +{<br>
> +     DRM_DEBUG("%s()\n", __func__);<br>
> +<br>
> +     __drm_atomic_helper_plane_destroy_state(state);<br>
> +     kfree(to_sprd_plane_state(state));<br>
> +}<br>
> +<br>
> +static int sprd_plane_atomic_set_property(struct drm_plane *plane,<br>
> +                                       struct drm_plane_state *state,<br>
> +                                       struct drm_property *property,<br>
> +                                       u64 val)<br>
> +{<br>
> +     struct sprd_plane *p = to_sprd_plane(plane);<br>
> +     struct sprd_plane_state *s = to_sprd_plane_state(state);<br>
> +<br>
> +     DRM_DEBUG("%s() name = %s, val = %llu\n",<br>
> +               __func__, property->name, val);<br>
> +<br>
> +     if (property == p->fbc_hsize_r_property)<br>
> +             s->fbc_hsize_r = val;<br>
> +     else if (property == p->fbc_hsize_y_property)<br>
> +             s->fbc_hsize_y = val;<br>
> +     else if (property == p->fbc_hsize_uv_property)<br>
> +             s->fbc_hsize_uv = val;<br>
> +     else if (property == p->y2r_coef_property)<br>
> +             s->y2r_coef = val;<br>
> +     else if (property == p->pallete_en_property)<br>
> +             s->pallete_en = val;<br>
> +     else if (property == p->pallete_color_property)<br>
> +             s->pallete_color = val;<br>
> +     else {<br>
> +             DRM_ERROR("property %s is invalid\n", property->name);<br>
> +             return -EINVAL;<br>
> +     }<br>
> +<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static int sprd_plane_atomic_get_property(struct drm_plane *plane,<br>
> +                                       const struct drm_plane_state *state,<br>
> +                                       struct drm_property *property,<br>
> +                                       u64 *val)<br>
> +{<br>
> +     struct sprd_plane *p = to_sprd_plane(plane);<br>
> +     const struct sprd_plane_state *s = to_sprd_plane_state(state);<br>
> +<br>
> +     DRM_DEBUG("%s() name = %s\n", __func__, property->name);<br>
> +<br>
> +     if (property == p->fbc_hsize_r_property)<br>
> +             *val = s->fbc_hsize_r;<br>
> +     else if (property == p->fbc_hsize_y_property)<br>
> +             *val = s->fbc_hsize_y;<br>
> +     else if (property == p->fbc_hsize_uv_property)<br>
> +             *val = s->fbc_hsize_uv;<br>
> +     else if (property == p->y2r_coef_property)<br>
> +             *val = s->y2r_coef;<br>
> +     else if (property == p->pallete_en_property)<br>
> +             *val = s->pallete_en;<br>
> +     else if (property == p->pallete_color_property)<br>
> +             *val = s->pallete_color;<br>
> +     else {<br>
> +             DRM_ERROR("property %s is invalid\n", property->name);<br>
> +             return -EINVAL;<br>
> +     }<br>
> +<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static const struct drm_plane_funcs sprd_plane_funcs = {<br>
> +     .update_plane = drm_atomic_helper_update_plane,<br>
> +     .disable_plane  = drm_atomic_helper_disable_plane,<br>
> +     .destroy = drm_plane_cleanup,<br>
> +     .reset = sprd_plane_reset,<br>
> +     .atomic_duplicate_state = sprd_plane_atomic_duplicate_state,<br>
> +     .atomic_destroy_state = sprd_plane_atomic_destroy_state,<br>
> +     .atomic_set_property = sprd_plane_atomic_set_property,<br>
> +     .atomic_get_property = sprd_plane_atomic_get_property,<br>
> +};<br>
> +<br>
> +static int sprd_plane_create_properties(struct sprd_plane *p, int index)<br>
> +{<br>
> +     struct drm_property *prop;<br>
> +     unsigned int supported_modes = BIT(DRM_MODE_BLEND_PIXEL_NONE) |<br>
> +                                    BIT(DRM_MODE_BLEND_PREMULTI) |<br>
> +                                    BIT(DRM_MODE_BLEND_COVERAGE);<br>
> +<br>
> +     /* create rotation property */<br>
> +     drm_plane_create_rotation_property(&p->plane,<br>
> +                                        DRM_MODE_ROTATE_0,<br>
> +                                        DRM_MODE_ROTATE_MASK |<br>
> +                                        DRM_MODE_REFLECT_MASK);<br>
> +<br>
> +     /* create alpha property */<br>
> +     drm_plane_create_alpha_property(&p->plane);<br>
> +<br>
> +     /* create blend mode property */<br>
> +     drm_plane_create_blend_mode_property(&p->plane, supported_modes);<br>
> +<br>
> +     /* create zpos property */<br>
> +     drm_plane_create_zpos_immutable_property(&p->plane, index);<br>
> +<br>
> +     /* create fbc header size property */<br>
> +     prop = drm_property_create_range(p-><a href="http://plane.dev" rel="noreferrer" target="_blank">plane.dev</a>, 0,<br>
> +                     "FBC header size RGB", 0, UINT_MAX);<br>
> +     if (!prop)<br>
> +             return -ENOMEM;<br>
> +     drm_object_attach_property(&p->plane.base, prop, 0);<br>
> +     p->fbc_hsize_r_property = prop;<br>
> +<br>
> +     prop = drm_property_create_range(p-><a href="http://plane.dev" rel="noreferrer" target="_blank">plane.dev</a>, 0,<br>
> +                     "FBC header size Y", 0, UINT_MAX);<br>
> +     if (!prop)<br>
> +             return -ENOMEM;<br>
> +     drm_object_attach_property(&p->plane.base, prop, 0);<br>
> +     p->fbc_hsize_y_property = prop;<br>
> +<br>
> +     prop = drm_property_create_range(p-><a href="http://plane.dev" rel="noreferrer" target="_blank">plane.dev</a>, 0,<br>
> +                     "FBC header size UV", 0, UINT_MAX);<br>
> +     if (!prop)<br>
> +             return -ENOMEM;<br>
> +     drm_object_attach_property(&p->plane.base, prop, 0);<br>
> +     p->fbc_hsize_uv_property = prop;<br>
> +<br>
> +     /* create y2r coef property */<br>
> +     prop = drm_property_create_range(p-><a href="http://plane.dev" rel="noreferrer" target="_blank">plane.dev</a>, 0,<br>
> +                     "YUV2RGB coef", 0, UINT_MAX);<br>
> +     if (!prop)<br>
> +             return -ENOMEM;<br>
> +     drm_object_attach_property(&p->plane.base, prop, 0);<br>
> +     p->y2r_coef_property = prop;<br>
> +<br>
> +     /* create pallete enable property */<br>
> +     prop = drm_property_create_range(p-><a href="http://plane.dev" rel="noreferrer" target="_blank">plane.dev</a>, 0,<br>
> +                     "pallete enable", 0, UINT_MAX);<br>
> +     if (!prop)<br>
> +             return -ENOMEM;<br>
> +     drm_object_attach_property(&p->plane.base, prop, 0);<br>
> +     p->pallete_en_property = prop;<br>
> +<br>
> +     /* create pallete color property */<br>
> +     prop = drm_property_create_range(p-><a href="http://plane.dev" rel="noreferrer" target="_blank">plane.dev</a>, 0,<br>
> +                     "pallete color", 0, UINT_MAX);<br>
> +     if (!prop)<br>
> +             return -ENOMEM;<br>
> +     drm_object_attach_property(&p->plane.base, prop, 0);<br>
> +     p->pallete_color_property = prop;<br>
> +<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +struct drm_plane *sprd_plane_init(struct drm_device *drm,<br>
> +                                     struct dpu_capability *cap,<br>
> +                                     enum drm_plane_type type)<br>
> +{<br>
> +     struct drm_plane *primary = NULL;<br>
> +     struct sprd_plane *p = NULL;<br>
> +     int err, i;<br>
> +<br>
> +     for (i = 0; i < cap->max_layers; i++) {<br>
> +<br>
> +             p = devm_kzalloc(drm->dev, sizeof(*p), GFP_KERNEL);<br>
> +             if (!p)<br>
> +                     return ERR_PTR(-ENOMEM);<br>
> +<br>
> +             err = drm_universal_plane_init(drm, &p->plane,<br>
> +                                            1 << drm->mode_config.num_crtc,<br>
> +                                            &sprd_plane_funcs, cap->fmts_ptr,<br>
> +                                            cap->fmts_cnt, NULL,<br>
> +                                            type, NULL);<br>
> +             if (err) {<br>
> +                     DRM_ERROR("fail to init primary plane\n");<br>
> +                     return ERR_PTR(err);<br>
> +             }<br>
> +<br>
> +             drm_plane_helper_add(&p->plane, &sprd_plane_helper_funcs);<br>
> +<br>
> +             sprd_plane_create_properties(p, i);<br>
> +<br>
> +             p->index = i;<br>
> +             if (i == 0)<br>
> +                     primary = &p->plane;<br>
> +     }<br>
> +<br>
> +     if (p)<br>
> +             DRM_INFO("dpu plane init ok\n");<br>
> +<br>
> +     return primary;<br>
> +}<br>
> diff --git a/drivers/gpu/drm/sprd/sprd_plane.h b/drivers/gpu/drm/sprd/sprd_plane.h<br>
> new file mode 100644<br>
> index 0000000..f63776a8<br>
> --- /dev/null<br>
> +++ b/drivers/gpu/drm/sprd/sprd_plane.h<br>
> @@ -0,0 +1,36 @@<br>
> +/* SPDX-License-Identifier: GPL-2.0 */<br>
> +/*<br>
> + * Copyright (C) 2019 Unisoc Inc.<br>
> + */<br>
> +<br>
> +#ifndef __SPRD_PLANE_H__<br>
> +#define __SPRD_PLANE_H__<br>
> +<br>
> +#define to_sprd_plane(x)             container_of(x, struct sprd_plane, plane)<br>
> +#define to_sprd_plane_state(x)       container_of(x, struct sprd_plane_state, state)<br>
> +<br>
> +struct sprd_plane_state {<br>
> +     struct drm_plane_state state;<br>
> +     u32 fbc_hsize_r;<br>
> +     u32 fbc_hsize_y;<br>
> +     u32 fbc_hsize_uv;<br>
> +     u32 y2r_coef;<br>
> +     u32 pallete_en;<br>
> +     u32 pallete_color;<br>
> +};<br>
> +<br>
> +struct sprd_plane {<br>
> +     struct drm_plane plane;<br>
> +     struct drm_property *fbc_hsize_r_property;<br>
> +     struct drm_property *fbc_hsize_y_property;<br>
> +     struct drm_property *fbc_hsize_uv_property;<br>
> +     struct drm_property *y2r_coef_property;<br>
> +     struct drm_property *pallete_en_property;<br>
> +     struct drm_property *pallete_color_property;<br>
> +     u32 index;<br>
> +};<br>
> +<br>
> +struct drm_plane *sprd_plane_init(struct drm_device *dev,<br>
> +                                     struct dpu_capability *cap,<br>
> +                                     enum drm_plane_type type);<br>
> +#endif<br>
> <br>
<br>
-- <br>
Thomas Zimmermann<br>
Graphics Driver Developer<br>
SUSE Software Solutions Germany GmbH<br>
Maxfeldstr. 5, 90409 Nürnberg, Germany<br>
(HRB 36809, AG Nürnberg)<br>
Geschäftsführer: Felix Imendörffer<br>
<br>
</blockquote></div></div>