Re: [PATCH v2 09/16] drm/imx: Add i.MX8qxp Display Controller KMS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 30, 2024 at 04:31:35PM GMT, Liu Ying wrote:
> On 07/28/2024, Dmitry Baryshkov wrote:
> > On Fri, Jul 12, 2024 at 05:32:36PM GMT, 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.
> > 
> > Generic comment: please consider moving dc_plane / dc_crtc defines to
> > the source files and dropping the headers.
> 
> struct dc_crtc is referenced from dc-drv.h and dc-kms.c.
> struct dc_plane is referenced from dc-crtc.c and dc-drv.h.
> 
> If no objections, I may drop dc-crtc.h and dc-plane.h,
> and move necessary stuff to dc-kms.h.
> 
> > 
> >>
> >> Signed-off-by: Liu Ying <victor.liu@xxxxxxx>
> >> ---
> >> v2:
> >> * Find next bridge from TCon's port.
> >> * Drop drm/drm_module.h include from dc-drv.c.
> >>
> >>  drivers/gpu/drm/imx/dc/Kconfig    |   2 +
> >>  drivers/gpu/drm/imx/dc/Makefile   |   5 +-
> >>  drivers/gpu/drm/imx/dc/dc-crtc.c  | 578 ++++++++++++++++++++++++++++++
> >>  drivers/gpu/drm/imx/dc/dc-crtc.h  |  67 ++++
> >>  drivers/gpu/drm/imx/dc/dc-de.h    |   3 +
> >>  drivers/gpu/drm/imx/dc/dc-drv.c   | 236 ++++++++++++
> >>  drivers/gpu/drm/imx/dc/dc-drv.h   |  21 ++
> >>  drivers/gpu/drm/imx/dc/dc-kms.c   | 143 ++++++++
> >>  drivers/gpu/drm/imx/dc/dc-kms.h   |  15 +
> >>  drivers/gpu/drm/imx/dc/dc-plane.c | 227 ++++++++++++
> >>  drivers/gpu/drm/imx/dc/dc-plane.h |  37 ++
> >>  11 files changed, 1332 insertions(+), 2 deletions(-)
> >>  create mode 100644 drivers/gpu/drm/imx/dc/dc-crtc.c
> >>  create mode 100644 drivers/gpu/drm/imx/dc/dc-crtc.h
> >>  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
> >>  create mode 100644 drivers/gpu/drm/imx/dc/dc-plane.h
> >>
> >> diff --git a/drivers/gpu/drm/imx/dc/Kconfig b/drivers/gpu/drm/imx/dc/Kconfig
> >> index b66b815fbdf1..dac0de009273 100644
> >> --- a/drivers/gpu/drm/imx/dc/Kconfig
> >> +++ b/drivers/gpu/drm/imx/dc/Kconfig
> >> @@ -1,6 +1,8 @@
> >>  config DRM_IMX8_DC
> >>  	tristate "Freescale i.MX8 Display Controller Graphics"
> >>  	depends on DRM && COMMON_CLK && OF && (ARCH_MXC || COMPILE_TEST)
> >> +	select DRM_GEM_DMA_HELPER
> >> +	select DRM_KMS_HELPER
> >>  	select GENERIC_IRQ_CHIP
> >>  	help
> >>  	  enable Freescale i.MX8 Display Controller(DC) graphics support
> >> diff --git a/drivers/gpu/drm/imx/dc/Makefile b/drivers/gpu/drm/imx/dc/Makefile
> >> index 1ce3e8a8db22..b9d33c074984 100644
> >> --- a/drivers/gpu/drm/imx/dc/Makefile
> >> +++ b/drivers/gpu/drm/imx/dc/Makefile
> >> @@ -1,6 +1,7 @@
> >>  # SPDX-License-Identifier: GPL-2.0
> >>  
> >> -imx8-dc-drm-objs := dc-cf.o dc-de.o dc-drv.o dc-ed.o dc-fg.o dc-fl.o dc-fu.o \
> >> -		    dc-fw.o dc-ic.o dc-lb.o dc-pe.o dc-tc.o
> >> +imx8-dc-drm-objs := dc-cf.o dc-crtc.o dc-de.o dc-drv.o dc-ed.o dc-fg.o dc-fl.o \
> >> +		    dc-fu.o dc-fw.o dc-ic.o dc-kms.o dc-lb.o dc-pe.o \
> >> +		    dc-plane.o dc-tc.o
> >>  
> >>  obj-$(CONFIG_DRM_IMX8_DC) += imx8-dc-drm.o
> >> diff --git a/drivers/gpu/drm/imx/dc/dc-crtc.c b/drivers/gpu/drm/imx/dc/dc-crtc.c
> >> new file mode 100644
> >> index 000000000000..e151e14a6677
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/imx/dc/dc-crtc.c
> >> @@ -0,0 +1,578 @@
> >> +// SPDX-License-Identifier: GPL-2.0+
> >> +/*
> >> + * Copyright 2024 NXP
> >> + */
> >> +
> >> +#include <linux/completion.h>
> >> +#include <linux/interrupt.h>
> >> +#include <linux/irqreturn.h>
> >> +#include <linux/pm_runtime.h>
> >> +#include <linux/spinlock.h>
> >> +
> >> +#include <drm/drm_atomic.h>
> >> +#include <drm/drm_atomic_helper.h>
> >> +#include <drm/drm_atomic_state_helper.h>
> >> +#include <drm/drm_crtc.h>
> >> +#include <drm/drm_device.h>
> >> +#include <drm/drm_drv.h>
> >> +#include <drm/drm_managed.h>
> >> +#include <drm/drm_modes.h>
> >> +#include <drm/drm_modeset_helper_vtables.h>
> >> +#include <drm/drm_plane.h>
> >> +#include <drm/drm_vblank.h>
> >> +
> >> +#include "dc-crtc.h"
> >> +#include "dc-de.h"
> >> +#include "dc-drv.h"
> >> +#include "dc-pe.h"
> >> +#include "dc-plane.h"
> >> +
> >> +#define DC_CRTC_WAIT_FOR_COMPLETION_TIMEOUT(c)				\
> >> +do {									\
> >> +	unsigned long ret;						\
> >> +	ret = wait_for_completion_timeout(&dc_crtc->c, HZ);		\
> >> +	if (ret == 0)							\
> >> +		dc_crtc_err(crtc, "%s: wait for " #c " timeout\n",	\
> >> +							__func__);	\
> >> +} while (0)
> >> +
> >> +#define DC_CRTC_CHECK_FRAMEGEN_FIFO(fg)					\
> >> +do {									\
> >> +	typeof(fg) _fg = (fg);						\
> >> +	if (dc_fg_secondary_requests_to_read_empty_fifo(_fg)) {		\
> >> +		dc_fg_secondary_clear_channel_status(_fg);		\
> >> +		dc_crtc_err(crtc, "%s: FrameGen FIFO empty\n",		\
> >> +							__func__);	\
> >> +	}								\
> >> +} while (0)
> >> +
> >> +#define DC_CRTC_WAIT_FOR_FRAMEGEN_SECONDARY_SYNCUP(fg)			\
> >> +do {									\
> >> +	if (dc_fg_wait_for_secondary_syncup(fg))			\
> >> +		dc_crtc_err(crtc,					\
> >> +			"%s: FrameGen secondary channel isn't syncup\n",\
> >> +							__func__);	\
> >> +} while (0)
> >> +
> >> +static u32 dc_crtc_get_vblank_counter(struct drm_crtc *crtc)
> >> +{
> >> +	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
> >> +
> >> +	return dc_fg_get_frame_index(dc_crtc->fg);
> >> +}
> >> +
> >> +static int dc_crtc_enable_vblank(struct drm_crtc *crtc)
> >> +{
> >> +	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
> >> +
> >> +	enable_irq(dc_crtc->irq_dec_framecomplete);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static void dc_crtc_disable_vblank(struct drm_crtc *crtc)
> >> +{
> >> +	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
> >> +
> >> +	disable_irq_nosync(dc_crtc->irq_dec_framecomplete);
> >> +}
> >> +
> >> +static irqreturn_t
> >> +dc_crtc_dec_framecomplete_irq_handler(int irq, void *dev_id)
> >> +{
> >> +	struct dc_crtc *dc_crtc = dev_id;
> >> +	struct drm_crtc *crtc = &dc_crtc->base;
> >> +	unsigned long flags;
> >> +
> >> +	drm_crtc_handle_vblank(crtc);
> >> +
> >> +	spin_lock_irqsave(&crtc->dev->event_lock, flags);
> >> +	if (dc_crtc->event) {
> >> +		drm_crtc_send_vblank_event(crtc, dc_crtc->event);
> >> +		dc_crtc->event = NULL;
> >> +		drm_crtc_vblank_put(crtc);
> >> +	}
> >> +	spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
> >> +
> >> +	return IRQ_HANDLED;
> >> +}
> >> +
> >> +static irqreturn_t dc_crtc_common_irq_handler(int irq, void *dev_id)
> >> +{
> >> +	struct dc_crtc *dc_crtc = dev_id;
> >> +	struct drm_crtc *crtc = &dc_crtc->base;
> >> +
> >> +	if (irq == dc_crtc->irq_dec_seqcomplete) {
> >> +		complete(&dc_crtc->dec_seqcomplete_done);
> >> +	} else if (irq == dc_crtc->irq_dec_shdld) {
> >> +		complete(&dc_crtc->dec_shdld_done);
> >> +	} else if (irq == dc_crtc->irq_ed_cont_shdld) {
> >> +		complete(&dc_crtc->ed_cont_shdld_done);
> >> +	} else if (irq == dc_crtc->irq_ed_safe_shdld) {
> >> +		complete(&dc_crtc->ed_safe_shdld_done);
> >> +	} else {
> >> +		dc_crtc_err(crtc, "invalid CRTC irq(%u)\n", irq);
> >> +		return IRQ_NONE;
> > 
> > And this is a counter-example to my previous questions. If you had 4
> > separate handlers, there would have been no need for the futile "invalid
> > CRTC" error, because it would not be possible at all.
> 
> Ok, will drop the else clause.  Thanks.
> 
> > 
> >> +	}
> >> +
> >> +	return IRQ_HANDLED;
> >> +}
> >> +
> >> +static const struct drm_crtc_funcs dc_crtc_funcs = {
> >> +	.reset			= drm_atomic_helper_crtc_reset,
> >> +	.destroy		= drm_crtc_cleanup,
> >> +	.set_config		= drm_atomic_helper_set_config,
> >> +	.page_flip		= drm_atomic_helper_page_flip,
> >> +	.atomic_duplicate_state	= drm_atomic_helper_crtc_duplicate_state,
> >> +	.atomic_destroy_state	= drm_atomic_helper_crtc_destroy_state,
> >> +	.get_vblank_counter	= dc_crtc_get_vblank_counter,
> >> +	.enable_vblank		= dc_crtc_enable_vblank,
> >> +	.disable_vblank		= dc_crtc_disable_vblank,
> >> +	.get_vblank_timestamp	= drm_crtc_vblank_helper_get_vblank_timestamp,
> >> +};
> >> +
> >> +static void dc_crtc_queue_state_event(struct drm_crtc_state *crtc_state)
> >> +{
> >> +	struct drm_crtc *crtc = crtc_state->crtc;
> >> +	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
> >> +
> >> +	spin_lock_irq(&crtc->dev->event_lock);
> >> +	if (crtc_state->event) {
> >> +		WARN_ON(drm_crtc_vblank_get(crtc));
> >> +		WARN_ON(dc_crtc->event);
> >> +		dc_crtc->event = crtc_state->event;
> >> +		crtc_state->event = NULL;
> >> +	}
> >> +	spin_unlock_irq(&crtc->dev->event_lock);
> >> +}
> >> +
> >> +static enum drm_mode_status
> >> +dc_crtc_check_clock(struct dc_crtc *dc_crtc, int clk_khz)
> >> +{
> >> +	return dc_fg_check_clock(dc_crtc->fg, clk_khz);
> >> +}
> >> +
> >> +static enum drm_mode_status
> >> +dc_crtc_mode_valid(struct drm_crtc *crtc, const struct drm_display_mode *mode)
> >> +{
> >> +	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
> >> +	enum drm_mode_status status;
> >> +
> >> +	status = dc_crtc_check_clock(dc_crtc, mode->clock);
> >> +	if (status != MODE_OK)
> >> +		return status;
> >> +
> >> +	if (mode->crtc_clock > DC_FRAMEGEN_MAX_CLOCK_KHZ)
> >> +		return MODE_CLOCK_HIGH;
> >> +
> >> +	return MODE_OK;
> >> +}
> >> +
> >> +static int
> >> +dc_crtc_atomic_check(struct drm_crtc *crtc, struct drm_atomic_state *state)
> >> +{
> >> +	struct drm_crtc_state *new_crtc_state =
> >> +				drm_atomic_get_new_crtc_state(state, crtc);
> >> +	struct drm_display_mode *adj = &new_crtc_state->adjusted_mode;
> >> +	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
> >> +	enum drm_mode_status status;
> >> +
> >> +	status = dc_crtc_check_clock(dc_crtc, adj->clock);
> >> +	if (status != MODE_OK)
> >> +		return -EINVAL;
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static void
> >> +dc_crtc_atomic_begin(struct drm_crtc *crtc, struct drm_atomic_state *state)
> >> +{
> >> +	struct drm_crtc_state *new_crtc_state =
> >> +				drm_atomic_get_new_crtc_state(state, crtc);
> >> +	struct dc_drm_device *dc_drm = to_dc_drm_device(crtc->dev);
> >> +	struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
> >> +	int idx, ret;
> >> +
> >> +	if (!drm_atomic_crtc_needs_modeset(new_crtc_state) ||
> >> +	    !new_crtc_state->active)
> >> +		return;
> > 
> > Why? Can it be called under such conditions?
> 
> This is needed to make sure the balance of calling
> pm_runtime_resume_and_get(dc_crtc->pe->dev)
> from dc_crtc_atomic_begin() and calling
> pm_runtime_put(dc_crtc->pe->dev)
> from dc_crtc_atomic_disable().
> 
> pm_runtime_resume_and_get(dc_crtc->pe->dev) is called
> only when the CRTC is to be enabled with a modeset
> commit.
> 
> > 
> >> +
> >> +	if (!drm_dev_enter(crtc->dev, &idx))
> >> +		return;
> > 
> > Can you please give an example of a driver using drm_dev_enter()/_exit()
> > in DRM callbacks?
> 
> vc4.
> 
> BTW, this is required by Maxime, as noted in cover letter.
> 
> > 
> >> +
> >> +	/* request pixel engine power-on when CRTC starts to be active */
> >> +	ret = pm_runtime_resume_and_get(dc_crtc->pe->dev);
> > 
> > This function doesn't return an error. So if pm_runtime_resume_and_get()
> 
> Kerneldoc of pm_runtime_resume_and_get() mentions error code.
> '
> or a negative error code otherwise
> '
> So, it may return an error.
> 
> > didn't increment the counter, corresponding pm_runtime_put() might cause
> > an underflow. Instead void functions should use pm_runtime_get_sync()
> 
> pm_runtime_resume_and_get() is called from dc_crtc_atomic_begin(), which
> is atomic considering the general DRM atomic KMS idea.  So, if the call
> returns an error, the best we can do is to print the error out like
> this driver does IMO.  The call should not fail in the first place due
> to the "atomic" sense, though it can fail in theory.
> 
> pm_runtime_get_sync() may also return an error. And it's Kerneldoc kinda
> says pm_runtime_resume_and_get() is better.
> '
> Consider using pm_runtime_resume_and_get() instead of it, especially
> if its return value is checked by the caller, as this is likely to result
> in cleaner code.
> '
> 
> > 
> > Also can any of the code running afterwards result in the unclocked
> > exception if resume fails?
> 
> Yes.  But, it's all atomic anyway...
> 
> > 
> >> +	if (ret)
> >> +		dc_crtc_err(crtc, "failed to get DC pixel engine RPM: %d\n",
> >> +			    ret);
> >> +
> >> +	atomic_inc(&dc_drm->pe_rpm_count);
> > 
> > Why do you need a separate RPM count? RPM code already has one.
> 
> If no objections, I will drop the count and call
> pm_runtime_active(dc_crtc->pe->dev) from dc_crtc_disable_at_unbind().

Why do you need the disable_at_unbind() thing? Doesn't shutdown take
care of disabling it for you?

> 
> Thanks.
> 
> > 
> >> +
> >> +	drm_dev_exit(idx);
> >> +}
> >> +
> > 
> > 
> > [...]
> > 
> >> +
> >> +static int
> >> +dc_crtc_request_irq(struct dc_crtc *dc_crtc, struct device *dev,
> >> +		    unsigned int irq,
> >> +		    irqreturn_t (*irq_handler)(int irq, void *dev_id))
> >> +{
> >> +	int ret;
> >> +
> >> +	ret = request_irq(irq, irq_handler, IRQF_NO_AUTOEN, dev_name(dev),
> >> +			  dc_crtc);
> >> +	if (ret < 0)
> >> +		dev_err(dev, "failed to request irq(%u): %d\n", irq, ret);
> >> +
> >> +	return ret;
> >> +}
> >> +
> >> +static int dc_crtc_request_irqs(struct drm_device *drm, struct dc_crtc *dc_crtc)
> >> +{
> >> +	struct {
> >> +		struct device *dev;
> >> +		unsigned int irq;
> >> +		irqreturn_t (*irq_handler)(int irq, void *dev_id);
> >> +	} irqs[] = {
> >> +		{
> >> +			dc_crtc->de->dev,
> >> +			dc_crtc->irq_dec_framecomplete,
> >> +			dc_crtc_dec_framecomplete_irq_handler,
> >> +		}, {
> >> +			dc_crtc->de->dev,
> >> +			dc_crtc->irq_dec_seqcomplete,
> >> +			dc_crtc_common_irq_handler,
> >> +		}, {
> >> +			dc_crtc->de->dev,
> >> +			dc_crtc->irq_dec_shdld,
> >> +			dc_crtc_common_irq_handler,
> >> +		}, {
> >> +			dc_crtc->ed_cont->dev,
> >> +			dc_crtc->irq_ed_cont_shdld,
> >> +			dc_crtc_common_irq_handler,
> >> +		}, {
> >> +			dc_crtc->ed_safe->dev,
> >> +			dc_crtc->irq_ed_safe_shdld,
> >> +			dc_crtc_common_irq_handler,
> >> +		},
> >> +	};
> >> +	struct drm_crtc *crtc = &dc_crtc->base;
> >> +	int i, ret;
> >> +
> >> +	dc_crtc->irqs = drmm_kcalloc(drm, ARRAY_SIZE(irqs),
> >> +				     sizeof(*dc_crtc->irqs), GFP_KERNEL);
> > 
> > Using array would remove the necessity to call drmm_kcalloc here().
> 
> Ok, I may use a macro to define the array size instead.
> 
> #define DC_CRTC_IRQS    5

Just embed the array into dc_crtc, no need for extra defines.

> 
> 
> > 
> >> +	if (!dc_crtc->irqs) {
> >> +		dev_err(drm->dev, "failed to allocate CRTC%u irqs\n",
> >> +			crtc->index);
> >> +		return -ENOMEM;
> >> +	}
> >> +
> >> +	for (i = 0; i < ARRAY_SIZE(irqs); i++) {
> >> +		struct dc_crtc_irq *irq = &dc_crtc->irqs[i];
> >> +
> >> +		ret = dc_crtc_request_irq(dc_crtc, irqs[i].dev, irqs[i].irq,
> >> +					  irqs[i].irq_handler);
> >> +		if (ret)
> >> +			return ret;
> >> +
> >> +		irq->dc_crtc = dc_crtc;
> >> +		irq->irq = irqs[i].irq;
> >> +
> >> +		ret = drmm_add_action_or_reset(drm, dc_crtc_free_irq, irq);
> > 
> > Can you use devm_request_irq() instead?
> 
> No.
> 
> The requested irqs would be freed too late as devm_of_platform_populate()
> is called early from dc_probe().  They would be freed later than the time
> point where irq domain is removed from dc_ic_unbind().  That would cause
> a kernel Oops as I tried.

Ohh, you are using drmm here. Please don't use drmm for IRQ domains.

> 
> > 
> >> +		if (ret)
> >> +			return ret;
> >> +	}
> >> +
> >> +	return 0;
> >> +}
> >> +
> > 
> > [...]
> > 
> >> +
> >> +static int dc_kms_init_encoder_per_crtc(struct dc_drm_device *dc_drm,
> >> +					int crtc_index)
> >> +{
> >> +	struct dc_crtc *dc_crtc = &dc_drm->dc_crtc[crtc_index];
> >> +	struct drm_device *drm = &dc_drm->base;
> >> +	struct drm_crtc *crtc = &dc_crtc->base;
> >> +	struct drm_connector *connector;
> >> +	struct device *dev = drm->dev;
> >> +	struct drm_encoder *encoder;
> >> +	struct device_node *remote;
> >> +	struct drm_bridge *bridge;
> >> +	int ret = 0;
> >> +
> >> +	remote = of_graph_get_remote_node(dc_crtc->de->tc->dev->of_node, 0, -1);
> >> +	if (!of_device_is_available(remote))
> >> +		goto out;
> >> +
> >> +	bridge = of_drm_find_bridge(remote);
> > 
> > drm_of_find_panel_or_bridge() instead.
> 
> Nope.
> 
> The first bridge is always pixel combiner according to SoC design.
> It can't be a panel.

So pass NULL as a panel pointer. At least it saves you from calling
of_graph_get_remote_node() manually.

> >> +	if (!bridge) {
> >> +		ret = -EPROBE_DEFER;
> >> +		dev_err_probe(dev, ret, "failed to find bridge for CRTC%u\n",
> >> +			      crtc->index);
> >> +		goto out;
> >> +	}
> >> +
> >> +	encoder = &dc_drm->encoder[crtc_index];
> >> +	ret = drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_NONE);
> >> +	if (ret) {
> >> +		dev_err(dev, "failed to initialize encoder for CRTC%u: %d\n",
> >> +			crtc->index, ret);
> >> +		goto out;
> >> +	}
> >> +
> >> +	encoder->possible_crtcs = drm_crtc_mask(crtc);
> >> +
> >> +	ret = drm_bridge_attach(encoder, bridge, NULL,
> >> +				DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> >> +	if (ret) {
> >> +		dev_err(dev,
> >> +			"failed to attach bridge to encoder for CRTC%u: %d\n",
> >> +			crtc->index, ret);
> >> +		goto out;
> >> +	}
> >> +
> >> +	connector = drm_bridge_connector_init(drm, encoder);
> >> +	if (IS_ERR(connector)) {
> >> +		ret = PTR_ERR(connector);
> >> +		dev_err(dev, "failed to init bridge connector for CRTC%u: %d\n",
> >> +			crtc->index, ret);
> >> +		goto out;
> >> +	}
> >> +
> >> +	ret = drm_connector_attach_encoder(connector, encoder);
> >> +	if (ret)
> >> +		dev_err(dev,
> >> +			"failed to attach encoder to connector for CRTC%u: %d\n",
> >> +			crtc->index, ret);
> >> +
> >> +out:
> >> +	of_node_put(remote);
> >> +	return ret;
> >> +}
> >> +
> >> +int dc_kms_init(struct dc_drm_device *dc_drm)
> >> +{
> >> +	struct drm_device *drm = &dc_drm->base;
> >> +	int ret, i;
> >> +
> >> +	ret = drmm_mode_config_init(drm);
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	atomic_set(&dc_drm->pe_rpm_count, 0);
> >> +
> >> +	drm->mode_config.min_width = 60;
> >> +	drm->mode_config.min_height = 60;
> >> +	drm->mode_config.max_width = 8192;
> >> +	drm->mode_config.max_height = 8192;
> >> +	drm->mode_config.funcs = &dc_drm_mode_config_funcs;
> >> +
> >> +	drm->vblank_disable_immediate = true;
> >> +	drm->max_vblank_count = DC_FRAMEGEN_MAX_FRAME_INDEX;
> >> +
> >> +	for (i = 0; i < DC_CRTCS; i++) {
> >> +		ret = dc_crtc_init(dc_drm, i);
> >> +		if (ret)
> >> +			return ret;
> >> +
> >> +		ret = dc_kms_init_encoder_per_crtc(dc_drm, i);
> >> +		if (ret)
> >> +			return ret;
> >> +	}
> >> +
> >> +	ret = drm_vblank_init(drm, DC_CRTCS);
> >> +	if (ret) {
> >> +		dev_err(drm->dev, "failed to init vblank support: %d\n", ret);
> >> +		return ret;
> >> +	}
> >> +
> >> +	drm_mode_config_reset(drm);
> >> +
> >> +	drm_kms_helper_poll_init(drm);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +void dc_kms_uninit(struct dc_drm_device *dc_drm)
> >> +{
> >> +	drm_kms_helper_poll_fini(&dc_drm->base);
> >> +}
> >> diff --git a/drivers/gpu/drm/imx/dc/dc-kms.h b/drivers/gpu/drm/imx/dc/dc-kms.h
> >> new file mode 100644
> >> index 000000000000..4f66b11c106a
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/imx/dc/dc-kms.h
> >> @@ -0,0 +1,15 @@
> >> +/* SPDX-License-Identifier: GPL-2.0+ */
> >> +/*
> >> + * Copyright 2024 NXP
> >> + */
> >> +
> >> +#ifndef __DC_KMS_H__
> >> +#define __DC_KMS_H__
> >> +
> >> +#include "dc-de.h"
> >> +
> >> +#define DC_CRTCS	DC_DISPLAYS
> >> +#define DC_ENCODERS	DC_DISPLAYS
> >> +#define DC_PRIMARYS	DC_DISPLAYS
> > 
> > If they are all equal, why do you need separate defines?
> 
> Hmm, just for meaningful macro names to make code easy to read.


[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux