On 12/27/2024, Dmitry Baryshkov wrote:
> 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?
You are right. I'll drop dc_plane_check_no_off_screen() in next version.
>
>>> With that in mind
>>>
>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
>>
>> Thanks.
>
>
>
--
Regards,
Liu Ying
