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().
>
> With that in mind
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
Thanks.
>
>
>> +
>> + return 0;
>> +}
>> +
--
Regards,
Liu Ying
