On Wed, 25 Dec 2024 at 09:18, Liu Ying <victor.liu@xxxxxxx> wrote:
>
> On 12/23/2024, Dmitry Baryshkov wrote:
> > On Mon, Dec 23, 2024 at 02:41:40PM +0800, Liu Ying wrote:
> >> i.MX8qxp Display Controller(DC) is comprised of three main components that
> >> include a blit engine for 2D graphics accelerations, display controller for
> >> display output processing, as well as a command sequencer. Add kernel
> >> mode setting support for the display controller part with two CRTCs and
> >> two primary planes(backed by FetchLayer and FetchWarp respectively). The
> >> registers of the display controller are accessed without command sequencer
> >> involved, instead just by using CPU. The command sequencer is supposed to
> >> be used by the blit engine.
> >>
> >> Reviewed-by: Maxime Ripard <mripard@xxxxxxxxxx>
> >> Signed-off-by: Liu Ying <victor.liu@xxxxxxx>
> >> ---
> >> v7:
> >> * Drop using typeof in macros and explicitly define variable types. (Dmitry)
> >> * Add a comment for disable_irq_nosync() to explain why _nosync. (Dmitry)
> >> * Inline dc_crtc_check_clock(). (Dmitry)
> >> * Use global drm_dc->pe. (Dmitry)
> >> * Drop dc_crtc_disable_at_unbind(). (Dmitry)
> >> * Add kernel doc for struct dc_{crtc,drm_device,plane}. (Dmitry)
> >> * Define common IRQ handlers separately for each IRQs. (Dmitry)
> >> * Rebase this patch upon next-20241220, so drop date entry from drm_driver
> >> and correctly include drm/clients/drm_client_setup.h.
> >> * Collect Maxime's R-b tag.
> >>
> >> v6:
> >> * No change.
> >>
> >> v5:
> >> * Replace .remove_new with .remove in dc-drv.c. (Uwe)
> >>
> >> v4:
> >> * Move dc_fg_displaymode(), dc_fg_panic_displaymode() and dc_lb_blendcontrol()
> >> function calls from KMS routine to initialization stage. (Dmitry)
> >> * Drop dc-crtc.h and dc-plane.h header files and move relevant defines to
> >> appropriate .h header files or .c source files. (Dmitry)
> >> * Drop futile "else" clause from dc_crtc_common_irq_handler(). (Dmitry)
> >> * Drop dc_drm->pe_rpm_count. (Dmitry)
> >> * Drop DC_{CRTCS,ENCODERS,PRIMARYS} macros and only use DC_DISPLAYS. (Dmitry)
> >> * Drop drmm_kcalloc() function call to allocate an array for storing IRQs.
> >> Instead, put it in struct dc_crtc. (Dmitry)
> >> * Call devm_request_irq() to request IRQs, instead of using drmm action.
> >> (Dmitry)
> >> * Call devm_drm_of_get_bridge() to find the next bridge. (Dmitry)
> >> * Select DRM_CLIENT_SELECTION due to rebase.
> >> * Select the missing DRM_DISPLAY_HELPER and DRM_BRIDGE_CONNECTOR.
> >> * Use DRM_FBDEV_DMA_DRIVER_OPS due to rebase.
> >> * Replace drm_fbdev_dma_setup() with drm_client_setup_with_fourcc() due to
> >> rebase.
> >> * Replace drmm_add_action_or_reset() with devm_add_action_or_reset() to
> >> register dc_drm_component_unbind_all() action.
> >> * Request interrupts in dc_crtc_post_init() after encoder initialization to
> >> make sure next bridge is found first.
> >>
> >> v3:
> >> * No change.
> >>
> >> v2:
> >> * Find next bridge from TCon's port.
> >> * Drop drm/drm_module.h include from dc-drv.c.
> >>
> >> drivers/gpu/drm/imx/dc/Kconfig | 5 +
> >> drivers/gpu/drm/imx/dc/Makefile | 5 +-
> >> drivers/gpu/drm/imx/dc/dc-crtc.c | 556 ++++++++++++++++++++++++++++++
> >> drivers/gpu/drm/imx/dc/dc-de.h | 3 +
> >> drivers/gpu/drm/imx/dc/dc-drv.c | 238 +++++++++++++
> >> drivers/gpu/drm/imx/dc/dc-drv.h | 22 ++
> >> drivers/gpu/drm/imx/dc/dc-kms.c | 143 ++++++++
> >> drivers/gpu/drm/imx/dc/dc-kms.h | 131 +++++++
> >> drivers/gpu/drm/imx/dc/dc-plane.c | 241 +++++++++++++
> >> 9 files changed, 1342 insertions(+), 2 deletions(-)
> >> create mode 100644 drivers/gpu/drm/imx/dc/dc-crtc.c
> >> create mode 100644 drivers/gpu/drm/imx/dc/dc-kms.c
> >> create mode 100644 drivers/gpu/drm/imx/dc/dc-kms.h
> >> create mode 100644 drivers/gpu/drm/imx/dc/dc-plane.c
> >>
> >
> >
> >> +
> >> +static int dc_plane_check_no_off_screen(struct drm_plane_state *state,
> >> + struct drm_crtc_state *crtc_state)
> >> +{
> >> + if (state->dst.x1 < 0 || state->dst.y1 < 0 ||
> >> + state->dst.x2 > crtc_state->adjusted_mode.hdisplay ||
> >> + state->dst.y2 > crtc_state->adjusted_mode.vdisplay) {
> >> + dc_plane_dbg(state->plane, "no off screen\n");
> >> + return -EINVAL;
> >> + }
> >
> > Nit: doesn't drm_atomic_helper_check_plane_state() ensure in this already?
>
> The can_position argument is set to true, though it is a bit unusual
> for primary plane. If can_position is false, I think this should be
> ensured already by drm_atomic_helper_check_plane_state().
The function still calls drm_rect_clip_scaled() to clip the
plane_state->dst, does it not?
> > With that in mind
> >
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
>
> Thanks.
--
With best wishes
Dmitry
