| Message ID | 20180618132242.8673-5-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Tomi, (CC'ing Jacopo Mondi for a comment about bus_formats in bridge drivers) Thank you for the patch. On Monday, 18 June 2018 16:22:37 EEST Tomi Valkeinen wrote: > This patch adds a new DRM driver for Texas Instruments DSS6 IP used on > Texas Instruments Keystone K2G SoC. The DSS6 IP is a major change to the > older DSS IP versions, which are supported by the omapdrm driver, and > while on higher level the DSS6 resembles the older DSS versions, the > registers and the internal pipelines differ a lot. > > DSS6 IP on K2G is a "ultra-light" version, and has only a single plane > and a single output. The driver will also support future DSS versions, > which support multiple planes and outputs, so the driver already has > support for those. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/gpu/drm/Kconfig | 2 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/tidss/Kconfig | 10 + > drivers/gpu/drm/tidss/Makefile | 11 + > drivers/gpu/drm/tidss/tidss_crtc.c | 390 +++++++ > drivers/gpu/drm/tidss/tidss_crtc.h | 49 + > drivers/gpu/drm/tidss/tidss_dispc.h | 145 +++ > drivers/gpu/drm/tidss/tidss_dispc6.c | 1450 +++++++++++++++++++++++++ > drivers/gpu/drm/tidss/tidss_dispc6.h | 109 ++ > drivers/gpu/drm/tidss/tidss_drv.c | 333 ++++++ > drivers/gpu/drm/tidss/tidss_drv.h | 41 + > drivers/gpu/drm/tidss/tidss_encoder.c | 101 ++ > drivers/gpu/drm/tidss/tidss_encoder.h | 22 + > drivers/gpu/drm/tidss/tidss_irq.c | 193 ++++ > drivers/gpu/drm/tidss/tidss_irq.h | 25 + > drivers/gpu/drm/tidss/tidss_kms.c | 85 ++ > drivers/gpu/drm/tidss/tidss_kms.h | 14 + > drivers/gpu/drm/tidss/tidss_plane.c | 186 ++++ > drivers/gpu/drm/tidss/tidss_plane.h | 25 + > 19 files changed, 3192 insertions(+) > create mode 100644 drivers/gpu/drm/tidss/Kconfig > create mode 100644 drivers/gpu/drm/tidss/Makefile > create mode 100644 drivers/gpu/drm/tidss/tidss_crtc.c > create mode 100644 drivers/gpu/drm/tidss/tidss_crtc.h > create mode 100644 drivers/gpu/drm/tidss/tidss_dispc.h > create mode 100644 drivers/gpu/drm/tidss/tidss_dispc6.c > create mode 100644 drivers/gpu/drm/tidss/tidss_dispc6.h > create mode 100644 drivers/gpu/drm/tidss/tidss_drv.c > create mode 100644 drivers/gpu/drm/tidss/tidss_drv.h > create mode 100644 drivers/gpu/drm/tidss/tidss_encoder.c > create mode 100644 drivers/gpu/drm/tidss/tidss_encoder.h > create mode 100644 drivers/gpu/drm/tidss/tidss_irq.c > create mode 100644 drivers/gpu/drm/tidss/tidss_irq.h > create mode 100644 drivers/gpu/drm/tidss/tidss_kms.c > create mode 100644 drivers/gpu/drm/tidss/tidss_kms.h > create mode 100644 drivers/gpu/drm/tidss/tidss_plane.c > create mode 100644 drivers/gpu/drm/tidss/tidss_plane.h [snip] > diff --git a/drivers/gpu/drm/tidss/Kconfig b/drivers/gpu/drm/tidss/Kconfig > new file mode 100644 > index 000000000000..818666db08a4 > --- /dev/null > +++ b/drivers/gpu/drm/tidss/Kconfig > @@ -0,0 +1,10 @@ > +config DRM_TIDSS Isn't the name TIDSS a bit too generic ? Previous platforms also had a DSS. > + tristate "DRM Support for TI Keystone" > + depends on DRM && OF > + depends on ARM || ARM64 || COMPILE_TEST > + select DRM_KMS_HELPER > + select DRM_KMS_CMA_HELPER > + select DRM_GEM_CMA_HELPER > + select VIDEOMODE_HELPERS > + help > + TI Keystone This is a bit short :) > diff --git a/drivers/gpu/drm/tidss/Makefile b/drivers/gpu/drm/tidss/Makefile > new file mode 100644 > index 000000000000..ee6b24db0441 > --- /dev/null > +++ b/drivers/gpu/drm/tidss/Makefile > @@ -0,0 +1,11 @@ > +# SPDX-License-Identifier: GPL-2.0 > + > +tidss-y := tidss_drv.o \ > + tidss_kms.o \ > + tidss_crtc.o \ > + tidss_plane.o \ > + tidss_encoder.o \ > + tidss_dispc6.o \ > + tidss_irq.o How about keeping these alphabetically sorted? > +obj-$(CONFIG_DRM_TIDSS) += tidss.o > diff --git a/drivers/gpu/drm/tidss/tidss_crtc.c > b/drivers/gpu/drm/tidss/tidss_crtc.c new file mode 100644 > index 000000000000..22c11f1e3318 > --- /dev/null > +++ b/drivers/gpu/drm/tidss/tidss_crtc.c > @@ -0,0 +1,390 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ > + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> > + */ > + > +#include <drm/drmP.h> > +#include <drm/drm_atomic.h> > +#include <drm/drm_atomic_helper.h> > +#include <drm/drm_crtc.h> > +#include <drm/drm_crtc_helper.h> > +#include <drm/drm_fb_cma_helper.h> > +#include <drm/drm_gem_cma_helper.h> > +#include <drm/drm_plane_helper.h> > +#include <uapi/linux/media-bus-format.h> Can't you include <linux/media-bus-format.h> ? I think the uapi/ prefix isn't meant to be used within the kernel. > + > +#include "tidss_crtc.h" > +#include "tidss_dispc.h" > +#include "tidss_drv.h" > +#include "tidss_irq.h" > + > +/* ------------------------------------------------------------------------ > + * Page Flip > + */ > + > +static void tidss_crtc_finish_page_flip(struct tidss_crtc *tcrtc) > +{ > + struct drm_device *ddev = tcrtc->crtc.dev; > + struct tidss_device *tidss = ddev->dev_private; > + struct drm_pending_vblank_event *event; > + unsigned long flags; > + bool busy; > + > + /* > + * New settings are taken into use at VFP, and GO bit is cleared at > + * the same time. This happens before the vertical blank interrupt. > + * So there is a small change that the driver sets GO bit after VFP, but > + * before vblank, and we have to check for that case here. > + */ > + busy = tidss->dispc_ops->vp_go_busy(tidss->dispc, tcrtc->hw_videoport); > + if (busy) > + return; > + > + spin_lock_irqsave(&ddev->event_lock, flags); > + > + event = tcrtc->event; > + tcrtc->event = NULL; > + > + if (!event) { > + spin_unlock_irqrestore(&ddev->event_lock, flags); > + return; > + } > + > + dev_dbg(ddev->dev, "%s\n", __func__); Debugging leftover ? > + drm_crtc_send_vblank_event(&tcrtc->crtc, event); > + > + spin_unlock_irqrestore(&ddev->event_lock, flags); > + > + drm_crtc_vblank_put(&tcrtc->crtc); > +} > + > +void tidss_crtc_vblank_irq(struct drm_crtc *crtc) > +{ > + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); > + > + drm_crtc_handle_vblank(crtc); > + > + tidss_crtc_finish_page_flip(tcrtc); Shouldn't page flip be completed at frame done time ? What's the relationship between the vblank and frame done IRQs ? > +} > + > +void tidss_crtc_framedone_irq(struct drm_crtc *crtc) > +{ > + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); > + > + complete(&tcrtc->framedone_completion); > +} > + > +void tidss_crtc_error_irq(struct drm_crtc *crtc, u64 irqstatus) > +{ > + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); > + > + dev_err_ratelimited(crtc->dev->dev, "CRTC%u SYNC LOST: (irq %llx)\n", > + tcrtc->hw_videoport, irqstatus); > +} > + > + Extra blank line. > +/* ------------------------------------------------------------------------ > + * CRTC Functions > + */ > + > +static int tidss_crtc_atomic_check(struct drm_crtc *crtc, > + struct drm_crtc_state *state) > +{ > + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); > + struct drm_device *ddev = crtc->dev; > + struct tidss_device *tidss = ddev->dev_private; > + const struct drm_display_mode *mode = &state->adjusted_mode; > + struct tidss_crtc_state *tcrtc_state = to_tidss_crtc_state(state); > + int r; > + > + dev_dbg(ddev->dev, "%s\n", __func__); > + > + if (!state->enable) > + return 0; > + > + r = tidss->dispc_ops->vp_check_config(tidss->dispc, tcrtc->hw_videoport, > + mode, > + tcrtc_state->bus_format, > + tcrtc_state->bus_flags); > + > + if (r == 0) > + return 0; > + > + dev_err(ddev->dev, "%s: failed (%ux%u pclk %u kHz)\n", > + __func__, mode->hdisplay, mode->vdisplay, mode->clock); I think we shouldn't give userspace yet another way to spam the kernel log. This should be a debugging message at most, but I believe we should just remove it, as it doesn't give much information. If you want to add debugging messages to help userspace find out what goes wrong, let's push them down to the dispc ops where you could print the cause of the error. > + return r; > +} > + > +static void tidss_crtc_atomic_begin(struct drm_crtc *crtc, > + struct drm_crtc_state *old_crtc_state) > +{ > + struct drm_device *ddev = crtc->dev; > + > + dev_dbg(ddev->dev, "%s\n", __func__); Is this useful? Same comment for the other debugging messages below that serve as function tracers. If the function becomes empty you could just drop it. > +} > + > +static void tidss_crtc_atomic_flush(struct drm_crtc *crtc, > + struct drm_crtc_state *old_crtc_state) > +{ > + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); > + struct drm_device *ddev = crtc->dev; > + struct tidss_device *tidss = ddev->dev_private; > + > + dev_dbg(ddev->dev, "%s, crtc enabled %d, event %p\n", > + __func__, tcrtc->enabled, crtc->state->event); > + > + /* Only flush the CRTC if it is currently enabled. */ > + if (!tcrtc->enabled) In atomic drivers state should be stored in state structures. You could check old_crtc_state for this and remove the enabled field in struct tidss_crtc. > + return; > + > + WARN_ON(tidss->dispc_ops->vp_go_busy(tidss->dispc, > + tcrtc->hw_videoport)); What will then happen ? > + // I think we always need the event to signal flip done > + WARN_ON(!crtc->state->event); When using drm_atomic_helper_commit() the helper allocates an event internal if none is requested by userspace, except for legacy cursor updates if the driver implements atomic_°async_check and atomic_async_commit. You will this always have an event. I think you can drop this WARN_ON(), the driver will crash later when dereferencing the event anyway. > + if (crtc->state->color_mgmt_changed) { > + struct drm_color_lut *lut = NULL; > + uint length = 0; unsigned int ? uint doesn't seem to be a common type in the kernel. > + > + if (crtc->state->gamma_lut) { > + lut = (struct drm_color_lut *) > + crtc->state->gamma_lut->data; > + length = crtc->state->gamma_lut->length / > + sizeof(*lut); > + } > + tidss->dispc_ops->vp_set_gamma(tidss->dispc, > + tcrtc->hw_videoport, > + lut, length); > + } > + > + WARN_ON(drm_crtc_vblank_get(crtc) != 0); When can this fail ? > + spin_lock_irq(&ddev->event_lock); > + tidss->dispc_ops->vp_go(tidss->dispc, tcrtc->hw_videoport); > + > + if (crtc->state->event) { > + tcrtc->event = crtc->state->event; > + crtc->state->event = NULL; > + } > + > + spin_unlock_irq(&ddev->event_lock); > +} > + > +static void tidss_crtc_atomic_enable(struct drm_crtc *crtc, > + struct drm_crtc_state *old_state) > +{ > + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); > + struct tidss_crtc_state *tcrtc_state = to_tidss_crtc_state(crtc->state); > + struct drm_device *ddev = crtc->dev; > + struct tidss_device *tidss = ddev->dev_private; > + const struct drm_display_mode *mode = &crtc->state->adjusted_mode; > + int r; > + > + dev_dbg(ddev->dev, "%s, event %p\n", __func__, crtc->state->event); > + > + tidss->dispc_ops->runtime_get(tidss->dispc); > + > + r = tidss->dispc_ops->vp_set_clk_rate(tidss->dispc, tcrtc->hw_videoport, > + mode->clock * 1000); > + WARN_ON(r); > + > + r = tidss->dispc_ops->vp_enable_clk(tidss->dispc, tcrtc->hw_videoport); > + WARN_ON(r); That doesn't seem to be good error handling :-/ > + > + /* Turn vertical blanking interrupt reporting on. */ > + drm_crtc_vblank_on(crtc); > + > + tcrtc->enabled = true; > + > + tidss->dispc_ops->vp_enable(tidss->dispc, tcrtc->hw_videoport, > + mode, > + tcrtc_state->bus_format, > + tcrtc_state->bus_flags); > + > + spin_lock_irq(&ddev->event_lock); > + > + if (crtc->state->event) { > + drm_crtc_send_vblank_event(crtc, crtc->state->event); > + crtc->state->event = NULL; > + } Why do you report vblank when enabling the CRTC ? > + spin_unlock_irq(&ddev->event_lock); > +} > + > +static void tidss_crtc_atomic_disable(struct drm_crtc *crtc, > + struct drm_crtc_state *old_state) > +{ > + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); > + struct drm_device *ddev = crtc->dev; > + struct tidss_device *tidss = ddev->dev_private; > + > + dev_dbg(ddev->dev, "%s, event %p\n", __func__, crtc->state->event); > + > + reinit_completion(&tcrtc->framedone_completion); > + > + tidss->dispc_ops->vp_disable(tidss->dispc, tcrtc->hw_videoport); > + > + if (!wait_for_completion_timeout(&tcrtc->framedone_completion, > + msecs_to_jiffies(500))) > + dev_err(tidss->dev, "Timeout waiting for framedone on crtc %d", > + tcrtc->hw_videoport); Without detailed knowledge of the hardware I can't tell for sure, but couldn't this be racy ? Could a vblank event be reported right before the reinit_completion() call, and ->vp_disable() called before the start of the next frame, resulting in no further vblank event being generated ? A wake_up()/wait_for_event() with an explicit test on whether a page flip is pending (if possible at the hardware level ?) might be better. > + spin_lock_irq(&ddev->event_lock); > + if (crtc->state->event) { > + drm_crtc_send_vblank_event(crtc, crtc->state->event); Is this needed only when waiting for completion times out, or is it needed in the general case ? I would assume the event to be signaled by the interrupt handler. > + crtc->state->event = NULL; > + } > + spin_unlock_irq(&ddev->event_lock); > + > + tcrtc->enabled = false; > + > + drm_crtc_vblank_off(crtc); > + > + tidss->dispc_ops->vp_disable_clk(tidss->dispc, tcrtc->hw_videoport); > + > + tidss->dispc_ops->runtime_put(tidss->dispc); > + > + spin_lock_irq(&crtc->dev->event_lock); > + if (crtc->state->event) { > + drm_crtc_send_vblank_event(crtc, crtc->state->event); > + crtc->state->event = NULL; A second one ? > + } > + spin_unlock_irq(&crtc->dev->event_lock); > +} > + > +static > +enum drm_mode_status tidss_crtc_mode_valid(struct drm_crtc *crtc, > + const struct drm_display_mode *mode) > +{ > + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); > + struct drm_device *ddev = crtc->dev; > + struct tidss_device *tidss = ddev->dev_private; > + > + return tidss->dispc_ops->vp_check_mode(tidss->dispc, > + tcrtc->hw_videoport, > + mode); > +} > + > +static const struct drm_crtc_helper_funcs crtc_helper_funcs = { > + .atomic_check = tidss_crtc_atomic_check, > + .atomic_begin = tidss_crtc_atomic_begin, > + .atomic_flush = tidss_crtc_atomic_flush, > + .atomic_enable = tidss_crtc_atomic_enable, > + .atomic_disable = tidss_crtc_atomic_disable, > + > + .mode_valid = tidss_crtc_mode_valid, > +}; > + > +static void tidss_crtc_reset(struct drm_crtc *crtc) > +{ > + if (crtc->state) > + __drm_atomic_helper_crtc_destroy_state(crtc->state); > + > + kfree(crtc->state); As you're testing for crtc->state above you could put this in the same code block. It would avoid a double check in the case where crtc->state is NULL. > + crtc->state = kzalloc(sizeof(struct tidss_crtc_state), GFP_KERNEL); Please don't rely on drm_crtc_state being the first field of tidss_crtc_state, it's not very error-proof. Also, sizeof(struct name) isn't favoured. If you stop relying on the field order you will need to add a variable for the custom state pointer, so you will be able to use sizeof(*var). > + if (crtc->state) > + crtc->state->crtc = crtc; > +} > + > +static struct drm_crtc_state * > +tidss_crtc_duplicate_state(struct drm_crtc *crtc) > +{ > + struct tidss_crtc_state *state, *current_state; > + > + if (WARN_ON(!crtc->state)) > + return NULL; Can this happen ? > + current_state = to_tidss_crtc_state(crtc->state); > + > + state = kmalloc(sizeof(*state), GFP_KERNEL); > + if (!state) > + return NULL; > + > + __drm_atomic_helper_crtc_duplicate_state(crtc, &state->base); > + > + state->bus_format = current_state->bus_format; > + state->bus_flags = current_state->bus_flags; > + > + return &state->base; > +} > + > +static int tidss_crtc_enable_vblank(struct drm_crtc *crtc) > +{ > + struct drm_device *ddev = crtc->dev; > + struct tidss_device *tidss = ddev->dev_private; > + > + dev_dbg(ddev->dev, "%s\n", __func__); > + > + tidss->dispc_ops->runtime_get(tidss->dispc); > + > + tidss_irq_enable_vblank(crtc); > + > + return 0; > +} > + > +static void tidss_crtc_disable_vblank(struct drm_crtc *crtc) > +{ > + struct drm_device *ddev = crtc->dev; > + struct tidss_device *tidss = ddev->dev_private; > + > + dev_dbg(ddev->dev, "%s\n", __func__); > + > + tidss_irq_disable_vblank(crtc); > + > + tidss->dispc_ops->runtime_put(tidss->dispc); > +} > + > +static const struct drm_crtc_funcs crtc_funcs = { > + .reset = tidss_crtc_reset, > + .destroy = drm_crtc_cleanup, > + .set_config = drm_atomic_helper_set_config, > + .page_flip = drm_atomic_helper_page_flip, > + .atomic_duplicate_state = tidss_crtc_duplicate_state, > + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, > + .enable_vblank = tidss_crtc_enable_vblank, > + .disable_vblank = tidss_crtc_disable_vblank, > +}; > + > +struct tidss_crtc *tidss_crtc_create(struct tidss_device *tidss, u32 > hw_videoport, > + struct drm_plane *primary, struct device_node *epnode) epnode isn't used.What do you need the epnode for ? > +{ > + struct tidss_crtc *tcrtc; > + struct drm_crtc *crtc; > + int ret; > + > + tcrtc = devm_kzalloc(tidss->dev, sizeof(*tcrtc), GFP_KERNEL); > + if (!tcrtc) > + return ERR_PTR(-ENOMEM); > + > + tcrtc->hw_videoport = hw_videoport; > + init_completion(&tcrtc->framedone_completion); > + > + crtc = &tcrtc->crtc; > + > + ret = drm_crtc_init_with_planes(tidss->ddev, crtc, primary, > + NULL, &crtc_funcs, NULL); > + if (ret < 0) > + return ERR_PTR(ret); > + > + drm_crtc_helper_add(crtc, &crtc_helper_funcs); > + > + /* The dispc API adapts to what ever size we ask from in no For multiline comments the first line usually contains just /* s/from in/form it/ ? > + * matter what HW supports. X-server assumes 256 element gamma > + * tables so lets use that. Size of HW gamma table can be > + * extracted with dispc_vp_gamma_size(). If it returns 0 > + * gamma table is not supprted. > + */ > + if (tidss->dispc_ops->vp_gamma_size(tidss->dispc, hw_videoport)) { > + uint gamma_lut_size = 256; unsigned int ? Same for other locations below, if any. > + > + drm_crtc_enable_color_mgmt(crtc, 0, false, gamma_lut_size); > + drm_mode_crtc_set_gamma_size(crtc, gamma_lut_size); > + } > + > + return tcrtc; > +} > diff --git a/drivers/gpu/drm/tidss/tidss_crtc.h > b/drivers/gpu/drm/tidss/tidss_crtc.h new file mode 100644 > index 000000000000..9f32d81e55d9 > --- /dev/null > +++ b/drivers/gpu/drm/tidss/tidss_crtc.h > @@ -0,0 +1,49 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ > + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> > + */ > + > +#ifndef __TIDSS_CRTC_H__ > +#define __TIDSS_CRTC_H__ > + > +#include <linux/wait.h> > +#include <linux/completion.h> > +#include <drm/drm_crtc.h> Let's try to keep headers alphabetically sorted ? > +#include "tidss_dispc.h" > + > +#define to_tidss_crtc(c) container_of((c), struct tidss_crtc, crtc) > + > +struct tidss_crtc { > + struct drm_crtc crtc; > + > + u32 hw_videoport; > + > + struct drm_pending_vblank_event *event; > + > + /* has crtc_atomic_enable been called? */ > + bool enabled; > + > + struct completion framedone_completion; > +}; > + > +#define to_tidss_crtc_state(x) container_of(x, struct tidss_crtc_state, > base) > + > +struct tidss_crtc_state { > + /* Must be first. */ > + struct drm_crtc_state base; > + > + u32 bus_format; > + u32 bus_flags; > +}; > + > +struct tidss_crtc *tidss_crtc_create(struct tidss_device *tidss, u32 > hw_videoport, > + struct drm_plane *primary, struct device_node *epnode); > + > + > +void tidss_crtc_vblank_irq(struct drm_crtc *crtc); > +void tidss_crtc_framedone_irq(struct drm_crtc *crtc); > +void tidss_crtc_error_irq(struct drm_crtc *crtc, u64 irqstatus); > + > +#endif > diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h > b/drivers/gpu/drm/tidss/tidss_dispc.h new file mode 100644 > index 000000000000..38ee3d75404e > --- /dev/null > +++ b/drivers/gpu/drm/tidss/tidss_dispc.h > @@ -0,0 +1,145 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ > + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> > + */ > + > +#ifndef __TIDSS_DISPC_H__ > +#define __TIDSS_DISPC_H__ > + > +struct tidss_device; > +struct dispc_device; > +struct drm_color_lut; > + You could try to keep structure declarations sorted too. > +#define DSS_MAX_CHANNELS 8 > +#define DSS_MAX_PLANES 8 > + > +/* > + * Based on the above 2 defines the bellow defines describe following > + * u64 IRQ bits: > + * > + * bit group |dev |mrg0|mrg1|mrg2|mrg3|mrg4|mrg5|mrg6|mrg7|plane > 0-7|<unused> | > + * bit use |Dfou|FEOL|FEOL|FEOL|FEOL|FEOL|FEOL|FEOL|FEOL|UUUU|UUUU| | | | > | | > + * bit number|0-3 |4-7 |8-11| 12-35 | 36-43 | > 44-63 | > + * > + * device bits: D = OCP error > + * WB bits: f = frame done wb, o = wb buffer overflow, > + * u = wb buffer uncomplete > + * vp bits: F = frame done, E = vsync even, O = vsync odd, L = sync lost > + * plane bits: U = fifo underflow > + */ > + > +#define DSS_IRQ_DEVICE_OCP_ERR BIT_ULL(0) > + > +#define DSS_IRQ_DEVICE_FRAMEDONEWB BIT_ULL(1) > +#define DSS_IRQ_DEVICE_WBBUFFEROVERFLOW BIT_ULL(2) > +#define DSS_IRQ_DEVICE_WBUNCOMPLETEERROR BIT_ULL(3) > +#define DSS_IRQ_DEVICE_WB_MASK GENMASK_ULL(3, 1) > + > +#define DSS_IRQ_VP_BIT_N(ch, bit) (4 + 4 * ch + bit) Please enclose macro arguments in parentheses. Doesn't checkpatch warn about that ? > +#define DSS_IRQ_PLANE_BIT_N(plane, bit) \ > + (DSS_IRQ_VP_BIT_N(DSS_MAX_CHANNELS, 0) + 1 * plane + bit) > + > +#define DSS_IRQ_VP_BIT(ch, bit) BIT_ULL(DSS_IRQ_VP_BIT_N(ch, bit)) > +#define DSS_IRQ_PLANE_BIT(plane, bit) BIT_ULL(DSS_IRQ_PLANE_BIT_N(plane, > bit)) + > +#define DSS_IRQ_VP_MASK(ch) \ > + GENMASK_ULL(DSS_IRQ_VP_BIT_N(ch, 3), DSS_IRQ_VP_BIT_N(ch, 0)) > +#define DSS_IRQ_PLANE_MASK(plane) \ > + GENMASK_ULL(DSS_IRQ_PLANE_BIT_N(plane, 0), DSS_IRQ_PLANE_BIT_N(plane, 0)) > + > +#define DSS_IRQ_VP_FRAME_DONE(ch) DSS_IRQ_VP_BIT(ch, 0) > +#define DSS_IRQ_VP_VSYNC_EVEN(ch) DSS_IRQ_VP_BIT(ch, 1) > +#define DSS_IRQ_VP_VSYNC_ODD(ch) DSS_IRQ_VP_BIT(ch, 2) > +#define DSS_IRQ_VP_SYNC_LOST(ch) DSS_IRQ_VP_BIT(ch, 3) > + > +#define DSS_IRQ_PLANE_FIFO_UNDERFLOW(plane) DSS_IRQ_PLANE_BIT(plane, 0) > + > +struct tidss_vp_info { > + u32 default_color; > +}; > + > +struct tidss_plane_info { This structure would be worth documenting. > + dma_addr_t paddr; > + dma_addr_t p_uv_addr; /* for NV12 format */ > + u16 fb_width; > + u16 width; > + u16 height; > + u32 fourcc; > + > + u16 pos_x; > + u16 pos_y; > + u16 out_width; /* if 0, out_width == width */ > + u16 out_height; /* if 0, out_height == height */ > + u8 global_alpha; > + u8 pre_mult_alpha; > + u8 zorder; > +}; > + > +struct dispc_ops { tidss_dispc_ops ? > + u64 (*read_and_clear_irqstatus)(struct dispc_device *dispc); > + void (*write_irqenable)(struct dispc_device *dispc, u64 enable); > + > + int (*runtime_get)(struct dispc_device *dispc); > + void (*runtime_put)(struct dispc_device *dispc); > + > + int (*get_num_planes)(struct dispc_device *dispc); > + int (*get_num_vps)(struct dispc_device *dispc); > + > + const char *(*plane_name)(struct dispc_device *dispc, > + u32 hw_plane); > + const char *(*vp_name)(struct dispc_device *dispc, > + u32 hw_videoport); Do the hardware IDs need to be exactly and explicitly 32-bit wide ? Can't we use unsigned int instead ? > + > + u32 (*get_memory_bandwidth_limit)(struct dispc_device *dispc); > + > + void (*vp_enable)(struct dispc_device *dispc, u32 hw_videoport, > + const struct drm_display_mode *mode, > + u32 bus_fmt, u32 bus_flags); > + > + void (*vp_disable)(struct dispc_device *dispc, u32 hw_videoport); > + > + bool (*vp_go_busy)(struct dispc_device *dispc, > + u32 hw_videoport); > + void (*vp_go)(struct dispc_device *dispc, u32 hw_videoport); > + > + void (*vp_setup)(struct dispc_device *dispc, u32 hw_videoport, > + const struct tidss_vp_info *info); > + > + enum drm_mode_status (*vp_check_mode)(struct dispc_device *dispc, > + u32 hw_videoport, > + const struct drm_display_mode *mode); How about calling this vp_mode_Valid to use the DRM terminology ? > + int (*vp_check_config)(struct dispc_device *dispc, u32 hw_videoport, > + const struct drm_display_mode *mode, > + u32 bus_fmt, u32 bus_flags); > + > + u32 (*vp_gamma_size)(struct dispc_device *dispc, > + u32 hw_videoport); > + void (*vp_set_gamma)(struct dispc_device *dispc, > + u32 hw_videoport, > + const struct drm_color_lut *lut, > + unsigned int length); > + > + int (*plane_enable)(struct dispc_device *dispc, u32 hw_plane, > + bool enable); > + int (*plane_setup)(struct dispc_device *dispc, u32 hw_plane, > + const struct tidss_plane_info *oi, > + u32 hw_videoport); > + > + int (*vp_set_clk_rate)(struct dispc_device *dispc, > + u32 hw_videoport, unsigned long rate); > + int (*vp_enable_clk)(struct dispc_device *dispc, u32 hw_videoport); > + void (*vp_disable_clk)(struct dispc_device *dispc, u32 hw_videoport); > + > + int (*runtime_suspend)(struct dispc_device *dispc); > + int (*runtime_resume)(struct dispc_device *dispc); > + > + void (*remove)(struct dispc_device *dispc); > + > + int (*modeset_init)(struct dispc_device *dispc); > +}; I think I would sort the functions by category: dispc, vp, planes, ... > + > +int dispc6_init(struct tidss_device *tidss); > + > +#endif > diff --git a/drivers/gpu/drm/tidss/tidss_dispc6.c > b/drivers/gpu/drm/tidss/tidss_dispc6.c new file mode 100644 > index 000000000000..7772d4484a6e > --- /dev/null > +++ b/drivers/gpu/drm/tidss/tidss_dispc6.c [snip] > diff --git a/drivers/gpu/drm/tidss/tidss_dispc6.h > b/drivers/gpu/drm/tidss/tidss_dispc6.h new file mode 100644 > index 000000000000..80197c812acd > --- /dev/null > +++ b/drivers/gpu/drm/tidss/tidss_dispc6.h I'll review the dispc6 code separately. [snip] > diff --git a/drivers/gpu/drm/tidss/tidss_drv.c > b/drivers/gpu/drm/tidss/tidss_drv.c new file mode 100644 > index 000000000000..8002766c4640 > --- /dev/null > +++ b/drivers/gpu/drm/tidss/tidss_drv.c > @@ -0,0 +1,333 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ > + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> > + */ > + > +#include <linux/console.h> > +#include <linux/of_device.h> > +#include <linux/pm_runtime.h> > + > +#include <drm/drmP.h> > +#include <drm/drm_atomic.h> > +#include <drm/drm_atomic_helper.h> > +#include <drm/drm_crtc.h> > +#include <drm/drm_crtc_helper.h> > +#include <drm/drm_fb_cma_helper.h> > +#include <drm/drm_fb_helper.h> > +#include <drm/drm_gem_cma_helper.h> > + > +#include "tidss_dispc.h" > +#include "tidss_drv.h" > +#include "tidss_irq.h" > +#include "tidss_kms.h" > + > +/* ------------------------------------------------------------------------ > + * Device Information > + */ > + > +static const struct tidss_features tidss_k2g_feats = { > + .dispc_init = dispc6_init, > +}; > + > +static const struct of_device_id tidss_of_table[] = { > + { .compatible = "ti,k2g-dss", .data = &tidss_k2g_feats }, > + { } > +}; > + How about moving this a bit closer to where tidss_of_table is used ? Just before the probe function possibly ? As you use block comments to mark sections, a section named "DRM Driver" would be useful here. > +DEFINE_DRM_GEM_CMA_FOPS(tidss_fops); > + > +static struct drm_driver tidss_driver = { > + .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME > + | DRIVER_ATOMIC | DRIVER_HAVE_IRQ, > + .gem_free_object_unlocked = drm_gem_cma_free_object, > + .gem_vm_ops = &drm_gem_cma_vm_ops, > + .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > + .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > + .gem_prime_import = drm_gem_prime_import, > + .gem_prime_export = drm_gem_prime_export, > + .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, > + .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, > + .gem_prime_vmap = drm_gem_cma_prime_vmap, > + .gem_prime_vunmap = drm_gem_cma_prime_vunmap, > + .gem_prime_mmap = drm_gem_cma_prime_mmap, > + .dumb_create = drm_gem_cma_dumb_create, > + .fops = &tidss_fops, > + .name = "tidss", > + .desc = "TI Keystone DSS", > + .date = "20180215", So this date will be remembers forever as the birthday of the driver :-) > + .major = 1, > + .minor = 0, > + > + .irq_preinstall = tidss_irq_preinstall, > + .irq_postinstall = tidss_irq_postinstall, > + .irq_handler = tidss_irq_handler, > + .irq_uninstall = tidss_irq_uninstall, I personally find the way the DRM framework manages IRQ to lead to more complex code than doing it manually. Up to you. > +}; > + > +#ifdef CONFIG_PM > +/* ------------------------------------------------------------------------ > + * Power management > + */ > + > +static int tidss_pm_runtime_suspend(struct device *dev) > +{ > + struct tidss_device *tidss = dev_get_drvdata(dev); > + > + dev_dbg(dev, "%s\n", __func__); > + > + return tidss->dispc_ops->runtime_suspend(tidss->dispc); > +} > + > +static int tidss_pm_runtime_resume(struct device *dev) > +{ > + struct tidss_device *tidss = dev_get_drvdata(dev); > + int r; > + > + dev_dbg(dev, "%s\n", __func__); > + > + r = tidss->dispc_ops->runtime_resume(tidss->dispc); > + if (r) > + return r; > + > + tidss_irq_resume(tidss->ddev); > + > + return 0; > +} > + > +#ifdef CONFIG_PM_SLEEP > +static int tidss_suspend(struct device *dev) > +{ > + struct tidss_device *tidss = dev_get_drvdata(dev); > + struct drm_atomic_state *state; > + > + drm_kms_helper_poll_disable(tidss->ddev); > + > + console_lock(); > + drm_fbdev_cma_set_suspend(tidss->fbdev, 1); > + console_unlock(); > + > + state = drm_atomic_helper_suspend(tidss->ddev); > + > + if (IS_ERR(state)) { > + console_lock(); > + drm_fbdev_cma_set_suspend(tidss->fbdev, 0); > + console_unlock(); > + > + drm_kms_helper_poll_enable(tidss->ddev); > + > + return PTR_ERR(state); > + } > + > + tidss->saved_state = state; We have drm_mode_config_helper_suspend() and drm_mode_config_helper_resume(), could they be used ? > + return 0; > +} > + > +static int tidss_resume(struct device *dev) > +{ > + struct tidss_device *tidss = dev_get_drvdata(dev); > + int ret = 0; > + > + if (tidss->saved_state) > + ret = drm_atomic_helper_resume(tidss->ddev, tidss->saved_state); > + > + console_lock(); > + drm_fbdev_cma_set_suspend(tidss->fbdev, 0); > + console_unlock(); > + > + drm_kms_helper_poll_enable(tidss->ddev); > + > + return ret; > +} > +#endif /* CONFIG_PM_SLEEP */ > + > +static const struct dev_pm_ops tidss_pm_ops = { > + .runtime_suspend = tidss_pm_runtime_suspend, > + .runtime_resume = tidss_pm_runtime_resume, > + SET_SYSTEM_SLEEP_PM_OPS(tidss_suspend, tidss_resume) > +}; > + > +#endif /* CONFIG_PM */ > + > +/* ------------------------------------------------------------------------ > + * Platform driver > + */ > + > +static int tidss_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct tidss_device *tidss; > + struct drm_device *ddev; > + int ret; > + int irq; > + > + dev_dbg(dev, "%s\n", __func__); > + > + tidss = devm_kzalloc(dev, sizeof(*tidss), GFP_KERNEL); > + if (tidss == NULL) > + return -ENOMEM; > + > + tidss->pdev = pdev; > + tidss->dev = dev; > + tidss->feat = of_device_get_match_data(dev); I'd spell it features instead of feat if that's not too long. > + > + platform_set_drvdata(pdev, tidss); > + > + ddev = drm_dev_alloc(&tidss_driver, dev); > + if (IS_ERR(ddev)) > + return PTR_ERR(ddev); > + > + tidss->ddev = ddev; > + ddev->dev_private = tidss; > + > + pm_runtime_enable(dev); > + > + ret = tidss->feat->dispc_init(tidss); > + if (ret) { > + dev_err(dev, "failed to initialize dispc: %d\n", ret); > + goto err_disable_pm; > + } > + > +#ifndef CONFIG_PM_SLEEP > + /* no PM, so force enable DISPC */ > + tidss->dispc_ops->runtime_resume(tidss->dispc); > +#endif This should really be hidden somewhere in the PM framework, it's pretty ugly otherwise :( Or should we depend on CONFIG_PM_SLEEP ? > + ret = tidss_modeset_init(tidss); > + if (ret < 0) { > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "failed to init DRM/KMS (%d)\n", ret); > + goto err_runtime_suspend; > + } > + > + irq = platform_get_irq(tidss->pdev, 0); > + if (irq < 0) { > + ret = irq; > + dev_err(dev, "platform_get_irq failed: %d\n", ret); > + goto err_modeset_cleanup; > + } > + > + ret = drm_irq_install(ddev, irq); > + if (ret) { > + dev_err(dev, "drm_irq_install failed: %d\n", ret); > + goto err_modeset_cleanup; > + } > + > +#ifdef CONFIG_DRM_FBDEV_EMULATION > + if (ddev->mode_config.num_connector) { > + struct drm_fbdev_cma *fbdev; > + > + fbdev = drm_fbdev_cma_init(ddev, 32, > + ddev->mode_config.num_connector); > + if (IS_ERR(fbdev)) { > + dev_err(tidss->dev, "fbdev init failed\n"); > + ret = PTR_ERR(fbdev); > + goto err_irq_uninstall; > + } Should this be a non-fatal error ? We can still use the display even if the fbdev emulation isn't available. > + tidss->fbdev = fbdev; > + } > +#endif > + > + drm_kms_helper_poll_init(ddev); > + > + ret = drm_dev_register(ddev, 0); > + if (ret) { > + dev_err(dev, "failed to register DRM device\n"); > + goto err_poll_fini; > + } > + > + dev_dbg(dev, "%s done\n", __func__); > + > + return 0; > + > +err_poll_fini: > + drm_kms_helper_poll_fini(ddev); > + > + if (tidss->fbdev) > + drm_fbdev_cma_fini(tidss->fbdev); > + > + drm_atomic_helper_shutdown(ddev); > + > +#ifdef CONFIG_DRM_FBDEV_EMULATION > +err_irq_uninstall: > +#endif > + drm_irq_uninstall(ddev); > + > +err_modeset_cleanup: > + drm_mode_config_cleanup(ddev); > + > +err_runtime_suspend: > +#ifndef CONFIG_PM_SLEEP > + /* no PM, so force disable DISPC */ > + tidss->dispc_ops->runtime_suspend(tidss->dispc); > +#endif > + > + tidss->dispc_ops->remove(tidss->dispc); > + > +err_disable_pm: > + pm_runtime_disable(dev); > + > + drm_dev_put(ddev); > + > + return ret; > +} > + > +static int tidss_remove(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct tidss_device *tidss = platform_get_drvdata(pdev); > + struct drm_device *ddev = tidss->ddev; > + > + dev_dbg(dev, "%s\n", __func__); > + > + drm_dev_unregister(ddev); > + > + drm_kms_helper_poll_fini(ddev); > + > + if (tidss->fbdev) > + drm_fbdev_cma_fini(tidss->fbdev); > + > + drm_atomic_helper_shutdown(ddev); > + > + drm_irq_uninstall(ddev); > + > + drm_mode_config_cleanup(ddev); > + > +#ifndef CONFIG_PM_SLEEP > + /* no PM, so force disable DISPC */ > + tidss->dispc_ops->runtime_suspend(tidss->dispc); > +#endif > + > + tidss->dispc_ops->remove(tidss->dispc); > + > + pm_runtime_disable(dev); > + > + drm_dev_put(ddev); > + > + dev_dbg(dev, "%s done\n", __func__); > + > + return 0; > +} > + > +MODULE_DEVICE_TABLE(of, tidss_of_table); > + > +static struct platform_driver tidss_platform_driver = { > + .probe = tidss_probe, > + .remove = tidss_remove, > + .driver = { > + .name = "tidss", > +#ifdef CONFIG_PM > + .pm = &tidss_pm_ops, > +#endif > + .of_match_table = tidss_of_table, > + .suppress_bind_attrs = true, > + }, > +}; > + > +module_platform_driver(tidss_platform_driver); > + > +MODULE_AUTHOR("Tomi Valkeinen <tomi.valkeinen@ti.com>"); > +MODULE_DESCRIPTION("TI Keystone DSS Driver"); > +MODULE_LICENSE("GPL v2"); > diff --git a/drivers/gpu/drm/tidss/tidss_drv.h > b/drivers/gpu/drm/tidss/tidss_drv.h new file mode 100644 > index 000000000000..fd1e767e9308 > --- /dev/null > +++ b/drivers/gpu/drm/tidss/tidss_drv.h > @@ -0,0 +1,41 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ > + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> > + */ > + > +#ifndef __TIDSS_DRV_H__ > +#define __TIDSS_DRV_H__ > + > +#include <linux/spinlock.h> > + > +struct tidss_device { > + struct device *dev; /* Underlying DSS device */ > + struct platform_device *pdev; /* Underlying DSS platform device */ Do we need to store both ? Can't we store dev only, and cast it to platform_device in the only location that needs it ? > + struct drm_device *ddev; /* DRM device for DSS */ > + > + struct drm_fbdev_cma *fbdev; > + > + struct dispc_device *dispc; > + const struct dispc_ops *dispc_ops; > + > + const struct tidss_features *feat; > + > + u32 num_crtcs; I think unsigned_int should be more than enough. > + struct drm_crtc *crtcs[8]; Do you plan to support platforms with 8 CRTCs ? > + > + u32 num_planes; > + struct drm_plane *planes[8]; > + > + spinlock_t wait_lock; /* protects the irq masks */ > + u64 irq_mask; /* enabled irqs in addition to wait_list */ > + u64 irq_uf_mask; /* underflow irq bits for all planes */ > + > + struct drm_atomic_state *saved_state; > +}; > + > +struct tidss_features { > + int (*dispc_init)(struct tidss_device *tidss); > +}; > + > +#endif > diff --git a/drivers/gpu/drm/tidss/tidss_encoder.c > b/drivers/gpu/drm/tidss/tidss_encoder.c new file mode 100644 > index 000000000000..fb7bc3fc0fe8 > --- /dev/null > +++ b/drivers/gpu/drm/tidss/tidss_encoder.c > @@ -0,0 +1,101 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ > + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> > + */ > + > +#include <linux/export.h> > + > +#include <drm/drmP.h> > +#include <drm/drm_crtc.h> > +#include <drm/drm_crtc_helper.h> > +#include <drm/drm_panel.h> > +#include <drm/drm_of.h> > + > +#include "tidss_drv.h" > +#include "tidss_encoder.h" > +#include "tidss_crtc.h" > + > +/* ------------------------------------------------------------------------ > + * Encoder > + */ > + > +static void tidss_encoder_disable(struct drm_encoder *encoder) > +{ > + struct drm_device *ddev = encoder->dev; > + > + dev_dbg(ddev->dev, "%s\n", __func__); > + Extra blank line. > +} > + > +static void tidss_encoder_enable(struct drm_encoder *encoder) > +{ > + struct drm_device *ddev = encoder->dev; > + > + dev_dbg(ddev->dev, "%s\n", __func__); > + Here too. > +} The encoder enable and disable functions are optional. > +static int tidss_encoder_atomic_check(struct drm_encoder *encoder, > + struct drm_crtc_state *crtc_state, > + struct drm_connector_state *conn_state) > +{ > + struct drm_device *ddev = encoder->dev; > + struct tidss_crtc_state *tcrtc_state = to_tidss_crtc_state(crtc_state); > + struct drm_display_info *di = &conn_state->connector->display_info; > + > + dev_dbg(ddev->dev, "%s\n", __func__); > + > + // XXX any cleaner way to set bus format and flags? Not that I know of :-/ Jacopo (CC'ed) started working on support for bus formats in bridge drivers, which you might be interested in. This being said, why do you need to store them in the CRTC state, can't you access them directly in the two locations where they're used ? > + tcrtc_state->bus_format = di->bus_formats[0]; > + tcrtc_state->bus_flags = di->bus_flags; > + > + return 0; > +} > + > +static void tidss_encoder_mode_set(struct drm_encoder *encoder, > + struct drm_crtc_state *crtc_state, > + struct drm_connector_state *conn_state) > +{ > + struct drm_device *ddev = encoder->dev; > + > + dev_dbg(ddev->dev, "%s\n", __func__); > + Extra blank line. > +} This function is optional too. > + > +static const struct drm_encoder_helper_funcs encoder_helper_funcs = { > + .atomic_mode_set = tidss_encoder_mode_set, > + .disable = tidss_encoder_disable, > + .enable = tidss_encoder_enable, > + .atomic_check = tidss_encoder_atomic_check, > +}; > + > +static const struct drm_encoder_funcs encoder_funcs = { > + .destroy = drm_encoder_cleanup, > +}; > + > +struct tidss_encoder *tidss_encoder_create(struct tidss_device *tidss, > + u32 encoder_type, u32 possible_crtcs) > +{ > + struct tidss_encoder *tenc; > + struct drm_encoder *enc; > + int ret; > + > + tenc = devm_kzalloc(tidss->dev, sizeof(*tenc), GFP_KERNEL); > + if (!tenc) > + return ERR_PTR(-ENOMEM); > + > + enc = &tenc->encoder; > + enc->possible_crtcs = possible_crtcs; > + > + ret = drm_encoder_init(tidss->ddev, enc, &encoder_funcs, > + encoder_type, NULL); > + if (ret < 0) > + return ERR_PTR(ret); > + > + drm_encoder_helper_add(enc, &encoder_helper_funcs); > + > + dev_dbg(tidss->dev, "Encoder create done\n"); > + > + return tenc; > +} > diff --git a/drivers/gpu/drm/tidss/tidss_encoder.h > b/drivers/gpu/drm/tidss/tidss_encoder.h new file mode 100644 > index 000000000000..f1bd05a3f611 > --- /dev/null > +++ b/drivers/gpu/drm/tidss/tidss_encoder.h > @@ -0,0 +1,22 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ > + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> > + */ > + > +#ifndef __TIDSS_ENCODER_H__ > +#define __TIDSS_ENCODER_H__ > + > +#include <drm/drm_encoder.h> > + > +struct tidss_device; > +struct tidss_crtc; > + > +struct tidss_encoder { > + struct drm_encoder encoder; > +}; > + Do you need a tidss_encoder structure at all ? > +struct tidss_encoder *tidss_encoder_create(struct tidss_device *tidss, > + u32 encoder_type, u32 possible_crtcs); > + > +#endif > diff --git a/drivers/gpu/drm/tidss/tidss_irq.c > b/drivers/gpu/drm/tidss/tidss_irq.c new file mode 100644 > index 000000000000..07c4e1f0924e > --- /dev/null > +++ b/drivers/gpu/drm/tidss/tidss_irq.c > @@ -0,0 +1,193 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ > + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> > + */ > + > +#include <drm/drmP.h> > + > +#include "tidss_irq.h" > +#include "tidss_drv.h" > +#include "tidss_dispc.h" > +#include "tidss_crtc.h" > +#include "tidss_plane.h" > + > +/* call with wait_lock and dispc runtime held */ > +static void tidss_irq_update(struct drm_device *ddev) > +{ > + struct tidss_device *tidss = ddev->dev_private; > + > + assert_spin_locked(&tidss->wait_lock); > + > + tidss->dispc_ops->write_irqenable(tidss->dispc, tidss->irq_mask); > +} > + > +void tidss_irq_enable_vblank(struct drm_crtc *crtc) > +{ > + struct drm_device *ddev = crtc->dev; > + struct tidss_device *tidss = ddev->dev_private; > + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); > + u32 hw_videoport = tcrtc->hw_videoport; > + unsigned long flags; > + > + spin_lock_irqsave(&tidss->wait_lock, flags); > + tidss->irq_mask |= DSS_IRQ_VP_VSYNC_EVEN(hw_videoport) | > + DSS_IRQ_VP_VSYNC_ODD(hw_videoport); > + tidss_irq_update(ddev); > + spin_unlock_irqrestore(&tidss->wait_lock, flags); > +} > + > +void tidss_irq_disable_vblank(struct drm_crtc *crtc) > +{ > + struct drm_device *ddev = crtc->dev; > + struct tidss_device *tidss = ddev->dev_private; > + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); > + u32 hw_videoport = tcrtc->hw_videoport; > + unsigned long flags; > + > + spin_lock_irqsave(&tidss->wait_lock, flags); > + tidss->irq_mask &= ~(DSS_IRQ_VP_VSYNC_EVEN(hw_videoport) | > + DSS_IRQ_VP_VSYNC_ODD(hw_videoport)); > + tidss_irq_update(ddev); > + spin_unlock_irqrestore(&tidss->wait_lock, flags); > +} > + > +static void tidss_irq_fifo_underflow(struct tidss_device *tidss, > + u64 irqstatus) > +{ > + static DEFINE_RATELIMIT_STATE(_rs, DEFAULT_RATELIMIT_INTERVAL, > + DEFAULT_RATELIMIT_BURST); > + unsigned int i; > + u64 masked; > + > + spin_lock(&tidss->wait_lock); > + masked = irqstatus & tidss->irq_uf_mask & tidss->irq_mask; Can you have irq_uf_mask bits that are not set in irq_mask ? > + spin_unlock(&tidss->wait_lock); > + > + if (!masked) > + return; > + > + if (!__ratelimit(&_rs)) > + return; > + > + DRM_ERROR("FIFO underflow on "); > + > + for (i = 0; i < DSS_MAX_PLANES; ++i) { > + if (masked & DSS_IRQ_PLANE_FIFO_UNDERFLOW(i)) > + pr_cont("%u:%s ", i, > + tidss->dispc_ops->plane_name(tidss->dispc, i)); > + } > + > + pr_cont("(%016llx)\n", irqstatus); > +} > + > +static void tidss_irq_ocp_error_handler(struct drm_device *ddev, > + u64 irqstatus) > +{ > + if (irqstatus & DSS_IRQ_DEVICE_OCP_ERR) > + dev_err_ratelimited(ddev->dev, "OCP error\n"); > +} > + > +irqreturn_t tidss_irq_handler(int irq, void *arg) > +{ > + struct drm_device *ddev = (struct drm_device *) arg; > + struct tidss_device *tidss = ddev->dev_private; > + unsigned int id; > + u64 irqstatus; > + > + if (WARN_ON(!ddev->irq_enabled)) > + return IRQ_NONE; > + > + irqstatus = tidss->dispc_ops->read_and_clear_irqstatus(tidss->dispc); > + > + for (id = 0; id < tidss->num_crtcs; id++) { > + struct drm_crtc *crtc = tidss->crtcs[id]; > + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); > + u32 hw_videoport = tcrtc->hw_videoport; > + > + if (irqstatus & (DSS_IRQ_VP_VSYNC_EVEN(hw_videoport) | > + DSS_IRQ_VP_VSYNC_ODD(hw_videoport))) > + tidss_crtc_vblank_irq(crtc); > + > + if (irqstatus & (DSS_IRQ_VP_FRAME_DONE(hw_videoport))) > + tidss_crtc_framedone_irq(crtc); > + > + if (irqstatus & DSS_IRQ_VP_SYNC_LOST(hw_videoport)) > + tidss_crtc_error_irq(crtc, irqstatus); > + } > + > + tidss_irq_ocp_error_handler(ddev, irqstatus); > + tidss_irq_fifo_underflow(tidss, irqstatus); > + > + return IRQ_HANDLED; > +} > + > +void tidss_irq_preinstall(struct drm_device *ddev) > +{ > + struct tidss_device *tidss = ddev->dev_private; > + > + spin_lock_init(&tidss->wait_lock); > + > + tidss->dispc_ops->runtime_get(tidss->dispc); > + > + tidss->dispc_ops->write_irqenable(tidss->dispc, 0); > + tidss->dispc_ops->read_and_clear_irqstatus(tidss->dispc); > + > + tidss->dispc_ops->runtime_put(tidss->dispc); > +} > + > +int tidss_irq_postinstall(struct drm_device *ddev) > +{ > + struct tidss_device *tidss = ddev->dev_private; > + unsigned int i; > + unsigned long flags; > + > + tidss->dispc_ops->runtime_get(tidss->dispc); > + > + spin_lock_irqsave(&tidss->wait_lock, flags); > + > + tidss->irq_mask = DSS_IRQ_DEVICE_OCP_ERR; > + > + tidss->irq_uf_mask = 0; > + for (i = 0; i < tidss->num_planes; ++i) { > + struct tidss_plane *tplane = to_tidss_plane(tidss->planes[i]); > + > + tidss->irq_uf_mask |= DSS_IRQ_PLANE_FIFO_UNDERFLOW(tplane->hw_plane_id); > + } > + tidss->irq_mask |= tidss->irq_uf_mask; > + > + for (i = 0; i < tidss->num_crtcs; ++i) { > + struct tidss_crtc *tcrtc = to_tidss_crtc(tidss->crtcs[i]); > + > + tidss->irq_mask |= DSS_IRQ_VP_SYNC_LOST(tcrtc->hw_videoport); > + > + tidss->irq_mask |= DSS_IRQ_VP_FRAME_DONE(tcrtc->hw_videoport); > + } > + > + tidss_irq_update(ddev); > + > + spin_unlock_irqrestore(&tidss->wait_lock, flags); > + > + tidss->dispc_ops->runtime_put(tidss->dispc); > + > + return 0; > +} > + > +void tidss_irq_uninstall(struct drm_device *ddev) > +{ > + struct tidss_device *tidss = ddev->dev_private; > + > + tidss->dispc_ops->runtime_get(tidss->dispc); > + tidss->dispc_ops->write_irqenable(tidss->dispc, 0); > + tidss->dispc_ops->runtime_put(tidss->dispc); > +} > + > +void tidss_irq_resume(struct drm_device *ddev) > +{ > + struct tidss_device *tidss = ddev->dev_private; > + unsigned long flags; > + > + spin_lock_irqsave(&tidss->wait_lock, flags); > + tidss_irq_update(ddev); > + spin_unlock_irqrestore(&tidss->wait_lock, flags); > +} > diff --git a/drivers/gpu/drm/tidss/tidss_irq.h > b/drivers/gpu/drm/tidss/tidss_irq.h new file mode 100644 > index 000000000000..e1507298c9d8 > --- /dev/null > +++ b/drivers/gpu/drm/tidss/tidss_irq.h > @@ -0,0 +1,25 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ > + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> > + */ > + > +#ifndef __TIDSS_IRQ_H__ > +#define __TIDSS_IRQ_H__ > + > +#include <linux/types.h> > + > +struct drm_crtc; > +struct drm_device; > + > +void tidss_irq_enable_vblank(struct drm_crtc *crtc); > +void tidss_irq_disable_vblank(struct drm_crtc *crtc); > + > +void tidss_irq_preinstall(struct drm_device *ddev); > +int tidss_irq_postinstall(struct drm_device *ddev); > +void tidss_irq_uninstall(struct drm_device *ddev); > +irqreturn_t tidss_irq_handler(int irq, void *arg); > + > +void tidss_irq_resume(struct drm_device *ddev); > + > +#endif > diff --git a/drivers/gpu/drm/tidss/tidss_kms.c > b/drivers/gpu/drm/tidss/tidss_kms.c new file mode 100644 > index 000000000000..cd003f1569cd > --- /dev/null > +++ b/drivers/gpu/drm/tidss/tidss_kms.c > @@ -0,0 +1,85 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ > + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> > + */ > + > +#include <drm/drm_atomic.h> > +#include <drm/drm_atomic_helper.h> > +#include <drm/drm_crtc_helper.h> > +#include <drm/drm_fb_helper.h> > +#include <drm/drm_fb_cma_helper.h> This is nearly alphabetically ordered :-) > +#include <drm/drm_gem_framebuffer_helper.h> > + > +#include "tidss_crtc.h" > +#include "tidss_drv.h" > +#include "tidss_encoder.h" > +#include "tidss_kms.h" > +#include "tidss_plane.h" > + > +static void tidss_atomic_commit_tail(struct drm_atomic_state *old_state) > +{ > + struct drm_device *ddev = old_state->dev; > + struct tidss_device *tidss = ddev->dev_private; > + > + dev_dbg(ddev->dev, "%s\n", __func__); > + > + tidss->dispc_ops->runtime_get(tidss->dispc); > + > + drm_atomic_helper_commit_modeset_disables(ddev, old_state); > + drm_atomic_helper_commit_planes(ddev, old_state, 0); > + drm_atomic_helper_commit_modeset_enables(ddev, old_state); > + > + drm_atomic_helper_commit_hw_done(old_state); > + drm_atomic_helper_wait_for_flip_done(ddev, old_state); > + > + drm_atomic_helper_cleanup_planes(ddev, old_state); > + > + tidss->dispc_ops->runtime_put(tidss->dispc); > +} > + > +static const struct drm_mode_config_helper_funcs mode_config_helper_funcs = > { + .atomic_commit_tail = tidss_atomic_commit_tail, > +}; > + > +static const struct drm_mode_config_funcs mode_config_funcs = { > + .fb_create = drm_gem_fb_create, > + .atomic_check = drm_atomic_helper_check, > + .atomic_commit = drm_atomic_helper_commit, > +}; > + > +int tidss_modeset_init(struct tidss_device *tidss) > +{ > + struct drm_device *ddev = tidss->ddev; > + int ret; > + int i; i never takes negative values, you can make it an unsigned int. > + > + dev_dbg(tidss->dev, "%s\n", __func__); > + > + drm_mode_config_init(ddev); > + > + ddev->mode_config.min_width = 8; > + ddev->mode_config.min_height = 8; > + ddev->mode_config.max_width = 8096; > + ddev->mode_config.max_height = 8096; > + ddev->mode_config.funcs = &mode_config_funcs; > + ddev->mode_config.helper_private = &mode_config_helper_funcs; > + > + ret = tidss->dispc_ops->modeset_init(tidss->dispc); I'm not too fond of hiding the DRM encoder and DRM bridge instantiation code in the dispc layer. Would there be a way to move it here ? > + if (ret) > + return ret; > + > + ret = drm_vblank_init(ddev, tidss->num_crtcs); > + if (ret) > + return ret; > + > + /* Start with vertical blanking interrupt reporting disabled. */ > + for (i = 0; i < tidss->num_crtcs; ++i) > + drm_crtc_vblank_reset(tidss->crtcs[i]); > + > + drm_mode_config_reset(ddev); > + > + dev_dbg(tidss->dev, "%s done\n", __func__); > + > + return 0; > +} > diff --git a/drivers/gpu/drm/tidss/tidss_kms.h > b/drivers/gpu/drm/tidss/tidss_kms.h new file mode 100644 > index 000000000000..927b57b05827 > --- /dev/null > +++ b/drivers/gpu/drm/tidss/tidss_kms.h > @@ -0,0 +1,14 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ > + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> > + */ > + > +#ifndef __TIDSS_KMS_H__ > +#define __TIDSS_KMS_H__ > + > +struct tidss_device; > + > +int tidss_modeset_init(struct tidss_device *rcdu); Now I wonder which driver you used as a model to develop this one ;-) > + > +#endif > diff --git a/drivers/gpu/drm/tidss/tidss_plane.c > b/drivers/gpu/drm/tidss/tidss_plane.c new file mode 100644 > index 000000000000..539c3ae706de > --- /dev/null > +++ b/drivers/gpu/drm/tidss/tidss_plane.c > @@ -0,0 +1,186 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ > + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> > + */ > + > +#include <drm/drmP.h> > +#include <drm/drm_atomic.h> > +#include <drm/drm_atomic_helper.h> > +#include <drm/drm_crtc.h> > +#include <drm/drm_crtc_helper.h> > +#include <drm/drm_fb_cma_helper.h> > +#include <drm/drm_gem_cma_helper.h> > +#include <drm/drm_plane_helper.h> > + > +#include "tidss_crtc.h" > +#include "tidss_drv.h" > +#include "tidss_plane.h" > + > +static int tidss_plane_atomic_check(struct drm_plane *plane, > + struct drm_plane_state *state) > +{ > + struct drm_device *ddev = plane->dev; > + struct drm_crtc_state *crtc_state; > + int ret; > + > + dev_dbg(ddev->dev, "%s\n", __func__); > + > + if (!state->crtc) { > + /* > + * The visible field is not reset by the DRM core but only > + * updated by drm_plane_helper_check_state(), set it manually. > + */ So it's at least two drivers suffering from this. Could we fix it in the DRM core ? > + state->visible = false; > + return 0; > + } > + > + crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc); > + if (IS_ERR(crtc_state)) > + return PTR_ERR(crtc_state); > + > + /* XXX TODO: check scaling via dispc_ops */ > + > + ret = drm_atomic_helper_check_plane_state(state, crtc_state, > + 0, > + INT_MAX, > + true, true); > + if (ret < 0) > + return ret; > + > + if (!state->visible) > + return 0; > + This doesn't seem to be needed. You could then just return drm_atomic_helper_check_plane_state(...);. > + return 0; > +} > + > +static void tidss_plane_atomic_update(struct drm_plane *plane, > + struct drm_plane_state *old_state) > +{ > + struct drm_device *ddev = plane->dev; > + struct tidss_device *tidss = ddev->dev_private; > + struct tidss_plane *tplane = to_tidss_plane(plane); > + struct tidss_plane_info info; > + int ret; > + u32 hw_videoport; How about moving these two down after the struct variables ? > + struct drm_plane_state *state = plane->state; > + struct drm_framebuffer *fb = state->fb; > + uint32_t x, y; u32 ? > + struct drm_gem_cma_object *gem; > + > + dev_dbg(ddev->dev, "%s\n", __func__); > + > + if (!state->visible) { > + tidss->dispc_ops->plane_enable(tidss->dispc, tplane->hw_plane_id, > + false); > + return; > + } This too might be a candidate for handling in the DRM core. > + hw_videoport = to_tidss_crtc(state->crtc)->hw_videoport; > + > + memset(&info, 0, sizeof(info)); You can get rid of this by zeroing the variable when declaring it. > + > + info.fourcc = fb->format->format; > + info.pos_x = state->crtc_x; > + info.pos_y = state->crtc_y; > + info.out_width = state->crtc_w; > + info.out_height = state->crtc_h; > + info.width = state->src_w >> 16; > + info.height = state->src_h >> 16; > + info.zorder = state->zpos; > + > + x = state->src_x >> 16; > + y = state->src_y >> 16; > + > + gem = drm_fb_cma_get_gem_obj(fb, 0); > + > + info.paddr = gem->paddr + fb->offsets[0] + x * fb->format->cpp[0] + > + y * fb->pitches[0]; > + > + info.fb_width = fb->pitches[0] / fb->format->cpp[0]; > + > + if (fb->format->num_planes == 2) { > + gem = drm_fb_cma_get_gem_obj(fb, 1); > + > + info.p_uv_addr = gem->paddr + > + fb->offsets[0] + Shouldn't this be offsets[1] ? > + (x * fb->format->cpp[0] / fb->format->hsub) + > + (y * fb->pitches[0] / fb->format->vsub); And if you use cpp[1] and pitches[1], can't you get rid of the divisions by hsub and vsub ? > + } > + > + ret = tidss->dispc_ops->plane_setup(tidss->dispc, tplane->hw_plane_id, > + &info, hw_videoport); > + > + if (ret) { > + dev_err(plane->dev->dev, "Failed to setup plane %d\n", > + tplane->hw_plane_id); We should really make sure this never happens by validating parameters at atomic check time. > + tidss->dispc_ops->plane_enable(tidss->dispc, tplane->hw_plane_id, > + false); > + return; > + } > + > + tidss->dispc_ops->plane_enable(tidss->dispc, tplane->hw_plane_id, true); > +} > + > +static void tidss_plane_atomic_disable(struct drm_plane *plane, > + struct drm_plane_state *old_state) > +{ > + struct drm_device *ddev = plane->dev; > + struct tidss_device *tidss = ddev->dev_private; > + struct tidss_plane *tplane = to_tidss_plane(plane); > + > + dev_dbg(ddev->dev, "%s\n", __func__); > + > + tidss->dispc_ops->plane_enable(tidss->dispc, tplane->hw_plane_id, false); > +} > + > +static const struct drm_plane_helper_funcs tidss_plane_helper_funcs = { > + .atomic_check = tidss_plane_atomic_check, > + .atomic_update = tidss_plane_atomic_update, > + .atomic_disable = tidss_plane_atomic_disable, > +}; > + > +static const struct drm_plane_funcs tidss_plane_funcs = { > + .update_plane = drm_atomic_helper_update_plane, > + .disable_plane = drm_atomic_helper_disable_plane, > + .reset = drm_atomic_helper_plane_reset, > + .destroy = drm_plane_cleanup, > + .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state, > + .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, > +}; > + > +struct tidss_plane *tidss_plane_create(struct tidss_device *tidss, > + u32 hw_plane_id, u32 plane_type, > + u32 crtc_mask, const u32 *formats, > + u32 num_formats) > +{ > + enum drm_plane_type type; > + uint32_t possible_crtcs; > + uint num_planes = tidss->dispc_ops->get_num_planes(tidss->dispc); unsigned int ? > + int ret; > + struct tidss_plane *tplane; > + > + tplane = devm_kzalloc(tidss->dev, sizeof(*tplane), GFP_KERNEL); > + if (!tplane) > + return ERR_PTR(-ENOMEM); > + > + tplane->hw_plane_id = hw_plane_id; > + > + possible_crtcs = crtc_mask; > + type = plane_type; You can use crtc_mask and plane_type directly below, no need for local variables. > + > + ret = drm_universal_plane_init(tidss->ddev, &tplane->plane, > + possible_crtcs, > + &tidss_plane_funcs, > + formats, num_formats, > + NULL, type, NULL); > + if (ret < 0) > + return ERR_PTR(ret); > + > + drm_plane_helper_add(&tplane->plane, &tidss_plane_helper_funcs); > + if (num_planes > 1) > + drm_plane_create_zpos_property(&tplane->plane, hw_plane_id, 0, > + num_planes - 1); > + > + return tplane; > +} > diff --git a/drivers/gpu/drm/tidss/tidss_plane.h > b/drivers/gpu/drm/tidss/tidss_plane.h new file mode 100644 > index 000000000000..d865e14cb5f9 > --- /dev/null > +++ b/drivers/gpu/drm/tidss/tidss_plane.h > @@ -0,0 +1,25 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ > + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> > + */ > + > +#ifndef __TIDSS_PLANE_H__ > +#define __TIDSS_PLANE_H__ > + > +#include "tidss_dispc.h" You don't need to pull the whole dispc header in here, you can just include the header for drm_plane and forward-declare struct tidss_device. > + > +#define to_tidss_plane(p) container_of((p), struct tidss_plane, plane) > + > +struct tidss_plane { > + struct drm_plane plane; > + > + u32 hw_plane_id; > + Extra blank line here. > +}; > + > +struct tidss_plane *tidss_plane_create(struct tidss_device *tidss, > + u32 hw_plane_id, u32 plane_type, > + u32 crtc_mask, const u32 *formats, > + u32 num_formats); > +#endif
Hi Laurent, On Mon, Jul 30, 2018 at 05:12:15PM +0300, Laurent Pinchart wrote: > Hi Tomi, > > (CC'ing Jacopo Mondi for a comment about bus_formats in bridge drivers) thanks for CC'ing me > > Thank you for the patch. > > On Monday, 18 June 2018 16:22:37 EEST Tomi Valkeinen wrote: > > This patch adds a new DRM driver for Texas Instruments DSS6 IP used on > > Texas Instruments Keystone K2G SoC. The DSS6 IP is a major change to the > > older DSS IP versions, which are supported by the omapdrm driver, and > > while on higher level the DSS6 resembles the older DSS versions, the > > registers and the internal pipelines differ a lot. > > > > DSS6 IP on K2G is a "ultra-light" version, and has only a single plane > > and a single output. The driver will also support future DSS versions, > > which support multiple planes and outputs, so the driver already has > > support for those. > > > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > > --- [snip] > > > +static int tidss_encoder_atomic_check(struct drm_encoder *encoder, > > + struct drm_crtc_state *crtc_state, > > + struct drm_connector_state *conn_state) > > +{ > > + struct drm_device *ddev = encoder->dev; > > + struct tidss_crtc_state *tcrtc_state = to_tidss_crtc_state(crtc_state); > > + struct drm_display_info *di = &conn_state->connector->display_info; > > + > > + dev_dbg(ddev->dev, "%s\n", __func__); > > + > > + // XXX any cleaner way to set bus format and flags? > > Not that I know of :-/ Jacopo (CC'ed) started working on support for bus > formats in bridge drivers, which you might be interested in. For reference the series Laurent's talking about is: https://lkml.org/lkml/2018/4/19/143 with these bits being the most relevant ones: [add DRM bridge helper to store the bus format] https://lkml.org/lkml/2018/4/19/164 [make use of those helpers in a bridge device] https://lkml.org/lkml/2018/4/19/161 For my understanding of issue here, more than a requirement for storing bus formats in bridges (the code here goes directly to the connector format) this is another driver betting on the first available media format reported by the next DRM pipeline component being the 'right' one. I've seen a few DRM drivers to be honest, but most of them would benefit from a proper format negotiation API implementation in DRM. Maybe I've been unlucky and that's actually a corner case? :) I tend to think it's not, but you people with more DRM experience could tell better than me, also considering the always growing complexity of video pipelines in embedded systems. Thanks j
Hi, On 30/07/18 17:12, Laurent Pinchart wrote: > Hi Tomi, > > (CC'ing Jacopo Mondi for a comment about bus_formats in bridge drivers) > > Thank you for the patch. Thanks for the review! Due to the length of the mail, I'll cut out all the easy bits, which I'll address in the next version. >> +config DRM_TIDSS > > Isn't the name TIDSS a bit too generic ? Previous platforms also had a DSS. Any suggestions? =) I'm calling the previous generations (DSS2-5) OMAP DSS, as they were mostly used on OMAP platforms, and the cases they're used on non-OMAP platforms (AM5 for example), it's still very similar to a version used on an OMAP platform. I don't have any good idea on what to call DSS6 and DSS7, so... "tidss". >> +void tidss_crtc_vblank_irq(struct drm_crtc *crtc) >> +{ >> + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); >> + >> + drm_crtc_handle_vblank(crtc); >> + >> + tidss_crtc_finish_page_flip(tcrtc); > > Shouldn't page flip be completed at frame done time ? What's the relationship > between the vblank and frame done IRQs ? vblank happens at each vertical blank, frame done happens when the output is disabled, and the ongoing frame has been sent. So, framedone is essentially the "final" vblank on an output. >> + return; >> + >> + WARN_ON(tidss->dispc_ops->vp_go_busy(tidss->dispc, >> + tcrtc->hw_videoport)); > > What will then happen ? Undefined behavior. Probably the new config doesn't get enabled. Then again, if we hit the warning, something has already gone wrong earlier. >> + // I think we always need the event to signal flip done >> + WARN_ON(!crtc->state->event); > > When using drm_atomic_helper_commit() the helper allocates an event internal > if none is requested by userspace, except for legacy cursor updates if the > driver implements atomic_°async_check and atomic_async_commit. You will this > always have an event. I think you can drop this WARN_ON(), the driver will > crash later when dereferencing the event anyway. Ok. >> + >> + if (crtc->state->gamma_lut) { >> + lut = (struct drm_color_lut *) >> + crtc->state->gamma_lut->data; >> + length = crtc->state->gamma_lut->length / >> + sizeof(*lut); >> + } >> + tidss->dispc_ops->vp_set_gamma(tidss->dispc, >> + tcrtc->hw_videoport, >> + lut, length); >> + } >> + >> + WARN_ON(drm_crtc_vblank_get(crtc) != 0); > > When can this fail ? I hope never, but I guess it's good to print something if it does fail. Perhaps dev_err is better here. >> +static void tidss_crtc_atomic_enable(struct drm_crtc *crtc, >> + struct drm_crtc_state *old_state) >> +{ >> + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); >> + struct tidss_crtc_state *tcrtc_state = to_tidss_crtc_state(crtc->state); >> + struct drm_device *ddev = crtc->dev; >> + struct tidss_device *tidss = ddev->dev_private; >> + const struct drm_display_mode *mode = &crtc->state->adjusted_mode; >> + int r; >> + >> + dev_dbg(ddev->dev, "%s, event %p\n", __func__, crtc->state->event); >> + >> + tidss->dispc_ops->runtime_get(tidss->dispc); >> + >> + r = tidss->dispc_ops->vp_set_clk_rate(tidss->dispc, tcrtc->hw_videoport, >> + mode->clock * 1000); >> + WARN_ON(r); >> + >> + r = tidss->dispc_ops->vp_enable_clk(tidss->dispc, tcrtc->hw_videoport); >> + WARN_ON(r); > > That doesn't seem to be good error handling :-/ No, it doesn't, but what can we do? I guess print an error and return. I don't know how to handle errors in the atomic commit phase where failure is not an option anymore. >> + >> + /* Turn vertical blanking interrupt reporting on. */ >> + drm_crtc_vblank_on(crtc); >> + >> + tcrtc->enabled = true; >> + >> + tidss->dispc_ops->vp_enable(tidss->dispc, tcrtc->hw_videoport, >> + mode, >> + tcrtc_state->bus_format, >> + tcrtc_state->bus_flags); >> + >> + spin_lock_irq(&ddev->event_lock); >> + >> + if (crtc->state->event) { >> + drm_crtc_send_vblank_event(crtc, crtc->state->event); >> + crtc->state->event = NULL; >> + } > > Why do you report vblank when enabling the CRTC ? Hmm, I'm not sure. I think the idea was to report that the fb is taken into use but thinking about it, it doesn't make sense. >> +static void tidss_crtc_atomic_disable(struct drm_crtc *crtc, >> + struct drm_crtc_state *old_state) >> +{ >> + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); >> + struct drm_device *ddev = crtc->dev; >> + struct tidss_device *tidss = ddev->dev_private; >> + >> + dev_dbg(ddev->dev, "%s, event %p\n", __func__, crtc->state->event); >> + >> + reinit_completion(&tcrtc->framedone_completion); >> + >> + tidss->dispc_ops->vp_disable(tidss->dispc, tcrtc->hw_videoport); >> + >> + if (!wait_for_completion_timeout(&tcrtc->framedone_completion, >> + msecs_to_jiffies(500))) >> + dev_err(tidss->dev, "Timeout waiting for framedone on crtc %d", >> + tcrtc->hw_videoport); > > Without detailed knowledge of the hardware I can't tell for sure, but couldn't > this be racy ? Could a vblank event be reported right before the > reinit_completion() call, and ->vp_disable() called before the start of the > next frame, resulting in no further vblank event being generated ? A > wake_up()/wait_for_event() with an explicit test on whether a page flip is > pending (if possible at the hardware level ?) might be better. This doesn't use vblank, but framedone. There's a single framedone when the output has finished sending the last frame. >> + spin_lock_irq(&ddev->event_lock); >> + if (crtc->state->event) { >> + drm_crtc_send_vblank_event(crtc, crtc->state->event); > > Is this needed only when waiting for completion times out, or is it needed in > the general case ? I would assume the event to be signaled by the interrupt > handler. We don't get a vblank when the output disables, but framedone. At the moment the framedone handler only completes the above completion, nothing else. >> + crtc->state->event = NULL; >> + } >> + spin_unlock_irq(&ddev->event_lock); >> + >> + tcrtc->enabled = false; >> + >> + drm_crtc_vblank_off(crtc); >> + >> + tidss->dispc_ops->vp_disable_clk(tidss->dispc, tcrtc->hw_videoport); >> + >> + tidss->dispc_ops->runtime_put(tidss->dispc); >> + >> + spin_lock_irq(&crtc->dev->event_lock); >> + if (crtc->state->event) { >> + drm_crtc_send_vblank_event(crtc, crtc->state->event); >> + crtc->state->event = NULL; > > A second one ? This must be a mistake... >> + crtc->state = kzalloc(sizeof(struct tidss_crtc_state), GFP_KERNEL); > > Please don't rely on drm_crtc_state being the first field of tidss_crtc_state, > it's not very error-proof. I thought that was the style used in DRM all around? >> +static struct drm_crtc_state * >> +tidss_crtc_duplicate_state(struct drm_crtc *crtc) >> +{ >> + struct tidss_crtc_state *state, *current_state; >> + >> + if (WARN_ON(!crtc->state)) >> + return NULL; > > Can this happen ? I don't know. drm_atomic_helper_crtc_duplicate_state() has the same code. >> + u64 (*read_and_clear_irqstatus)(struct dispc_device *dispc); >> + void (*write_irqenable)(struct dispc_device *dispc, u64 enable); >> + >> + int (*runtime_get)(struct dispc_device *dispc); >> + void (*runtime_put)(struct dispc_device *dispc); >> + >> + int (*get_num_planes)(struct dispc_device *dispc); >> + int (*get_num_vps)(struct dispc_device *dispc); >> + >> + const char *(*plane_name)(struct dispc_device *dispc, >> + u32 hw_plane); >> + const char *(*vp_name)(struct dispc_device *dispc, >> + u32 hw_videoport); > > Do the hardware IDs need to be exactly and explicitly 32-bit wide ? Can't we > use unsigned int instead ? We can, it just makes the lines awfully long =). We could use u8 as well. >> +#ifdef CONFIG_PM_SLEEP >> +static int tidss_suspend(struct device *dev) >> +{ >> + struct tidss_device *tidss = dev_get_drvdata(dev); >> + struct drm_atomic_state *state; >> + >> + drm_kms_helper_poll_disable(tidss->ddev); >> + >> + console_lock(); >> + drm_fbdev_cma_set_suspend(tidss->fbdev, 1); >> + console_unlock(); >> + >> + state = drm_atomic_helper_suspend(tidss->ddev); >> + >> + if (IS_ERR(state)) { >> + console_lock(); >> + drm_fbdev_cma_set_suspend(tidss->fbdev, 0); >> + console_unlock(); >> + >> + drm_kms_helper_poll_enable(tidss->ddev); >> + >> + return PTR_ERR(state); >> + } >> + >> + tidss->saved_state = state; > > We have drm_mode_config_helper_suspend() and drm_mode_config_helper_resume(), > could they be used ? Perhaps. I didn't notice those, as I used v4.14 as a base for the first versions, and 4.14 didn't have them. >> + >> + platform_set_drvdata(pdev, tidss); >> + >> + ddev = drm_dev_alloc(&tidss_driver, dev); >> + if (IS_ERR(ddev)) >> + return PTR_ERR(ddev); >> + >> + tidss->ddev = ddev; >> + ddev->dev_private = tidss; >> + >> + pm_runtime_enable(dev); >> + >> + ret = tidss->feat->dispc_init(tidss); >> + if (ret) { >> + dev_err(dev, "failed to initialize dispc: %d\n", ret); >> + goto err_disable_pm; >> + } >> + >> +#ifndef CONFIG_PM_SLEEP >> + /* no PM, so force enable DISPC */ >> + tidss->dispc_ops->runtime_resume(tidss->dispc); >> +#endif > > This should really be hidden somewhere in the PM framework, it's pretty ugly > otherwise :( Or should we depend on CONFIG_PM_SLEEP ? Yes, it's ugly, and took some time to figure out why things are not working (if I recall right, mainline k2g config didn't have PM_SLEEP)... I don't think we should depend on PM_SLEEP. >> + ret = tidss_modeset_init(tidss); >> + if (ret < 0) { >> + if (ret != -EPROBE_DEFER) >> + dev_err(dev, "failed to init DRM/KMS (%d)\n", ret); >> + goto err_runtime_suspend; >> + } >> + >> + irq = platform_get_irq(tidss->pdev, 0); >> + if (irq < 0) { >> + ret = irq; >> + dev_err(dev, "platform_get_irq failed: %d\n", ret); >> + goto err_modeset_cleanup; >> + } >> + >> + ret = drm_irq_install(ddev, irq); >> + if (ret) { >> + dev_err(dev, "drm_irq_install failed: %d\n", ret); >> + goto err_modeset_cleanup; >> + } >> + >> +#ifdef CONFIG_DRM_FBDEV_EMULATION >> + if (ddev->mode_config.num_connector) { >> + struct drm_fbdev_cma *fbdev; >> + >> + fbdev = drm_fbdev_cma_init(ddev, 32, >> + ddev->mode_config.num_connector); >> + if (IS_ERR(fbdev)) { >> + dev_err(tidss->dev, "fbdev init failed\n"); >> + ret = PTR_ERR(fbdev); >> + goto err_irq_uninstall; >> + } > > Should this be a non-fatal error ? We can still use the display even if the > fbdev emulation isn't available. Technically yes, but things are probably pretty broken if fbdev setup fails. We could even proceed without irqs, and use polling =). I think it's better to just stop if things fail that are not supposed to fail. >> +struct tidss_device { >> + struct device *dev; /* Underlying DSS device */ >> + struct platform_device *pdev; /* Underlying DSS platform device */ > > Do we need to store both ? Can't we store dev only, and cast it to > platform_device in the only location that needs it ? Yes, I think I used it just as a shortcut. >> +static int tidss_encoder_atomic_check(struct drm_encoder *encoder, >> + struct drm_crtc_state *crtc_state, >> + struct drm_connector_state *conn_state) >> +{ >> + struct drm_device *ddev = encoder->dev; >> + struct tidss_crtc_state *tcrtc_state = to_tidss_crtc_state(crtc_state); >> + struct drm_display_info *di = &conn_state->connector->display_info; >> + >> + dev_dbg(ddev->dev, "%s\n", __func__); >> + >> + // XXX any cleaner way to set bus format and flags? > > Not that I know of :-/ Jacopo (CC'ed) started working on support for bus > formats in bridge drivers, which you might be interested in. > > This being said, why do you need to store them in the CRTC state, can't you > access them directly in the two locations where they're used ? If I recall right, it wasn't trivial to get that info in the crtc side. >> +static void tidss_irq_fifo_underflow(struct tidss_device *tidss, >> + u64 irqstatus) >> +{ >> + static DEFINE_RATELIMIT_STATE(_rs, DEFAULT_RATELIMIT_INTERVAL, >> + DEFAULT_RATELIMIT_BURST); >> + unsigned int i; >> + u64 masked; >> + >> + spin_lock(&tidss->wait_lock); >> + masked = irqstatus & tidss->irq_uf_mask & tidss->irq_mask; > > Can you have irq_uf_mask bits that are not set in irq_mask ? I think it's possible, at least in theory. I'm not sure if we can have such a case in practice with the current driver. In any case, I'd rather only look at irq bits that we've explicitly interested in (irq_mask), instead of trusting that no extra irq bits are present. >> + ddev->mode_config.min_width = 8; >> + ddev->mode_config.min_height = 8; >> + ddev->mode_config.max_width = 8096; >> + ddev->mode_config.max_height = 8096; >> + ddev->mode_config.funcs = &mode_config_funcs; >> + ddev->mode_config.helper_private = &mode_config_helper_funcs; >> + >> + ret = tidss->dispc_ops->modeset_init(tidss->dispc); > > I'm not too fond of hiding the DRM encoder and DRM bridge instantiation code > in the dispc layer. Would there be a way to move it here ? Well... Don't think of it as dispc layer, but as dss6/dss7 helper. The setup of the crtcs, planes, encoders, connectors and bridges depend on the HW, so it was much easier to have dss6 and dss7 specific setups, instead of trying to create common code for them (which I did first). We could do it here, either as a single common code, or separete functions similar to what's in dispcs now, but then it would mean exposing HW details (say, port types, how crtcs and encoders connect to each other, which planes can be used on which crtc) from the dispc side only for the purpose of modeset init. Those details are, of course, available afterwards via standard DRM ways, but until we've done the modeset init, we have those details only in custom form. Is there something specific that you dislike with this approach? My thinking was that the dispc6/7 files are essentially helper functions for things that are not common between DSS versions. From my omapdrm experiences, common functions for different DSS versions easily become a nightmare. Tomi
Hi Laurent, Tomi is busy with other things so I have taken over applying these comments. The rest is more or less clear, or commented by Tomi, but this is something have not addressed: On 30/07/18 17:12, Laurent Pinchart wrote: >> +static void tidss_crtc_atomic_flush(struct drm_crtc *crtc, >> + struct drm_crtc_state *old_crtc_state) >> +{ >> + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); >> + struct drm_device *ddev = crtc->dev; >> + struct tidss_device *tidss = ddev->dev_private; >> + >> + dev_dbg(ddev->dev, "%s, crtc enabled %d, event %p\n", >> + __func__, tcrtc->enabled, crtc->state->event); >> + >> + /* Only flush the CRTC if it is currently enabled. */ >> + if (!tcrtc->enabled) > In atomic drivers state should be stored in state structures. You could check > old_crtc_state for this and remove the enabled field in struct tidss_crtc. > The variable is need for tracking the HW state trough the state transition. I do not know which state variable I should use to keep that state information stored trough the process where one state changes into another. The drm_crtc_state already contains couple of variables describing whether crtc is enabled or not, or if the mode is going to change in the state transition (giving a hint that crtc is going go through disable-enable cycle). I tried to use all of those, and the old state variable, to accomplish the same behaviour as the current code has, but I could not. One of the problematic cases was a new drm client making an atomic commit, the old one being bf-console, with the same mode as the one was using. In that situation the crtc goes trough disable-enable cycle, but I could not find any way to detect the situation from the old and new crtc state. Enable-disable cycle means that we should not flip the go-bit, but just configure everything and enable the crtc, e.g skip the atomic flush and wait for enable instead. In any case this is for HW state, not for DRM state tracking. I could add a call back to dispc ops for asking if the video port is enabled and use that instead if you think that is more formally correct. Best regards, Jyri
Hi Jyri, (CC'ing Daniel Vetter) On Wednesday, 31 October 2018 18:24:28 EET Jyri Sarha wrote: > Hi Laurent, > Tomi is busy with other things so I have taken over applying these > comments. The rest is more or less clear, or commented by Tomi, but this > is something have not addressed: > > On 30/07/18 17:12, Laurent Pinchart wrote: > >> +static void tidss_crtc_atomic_flush(struct drm_crtc *crtc, > >> + struct drm_crtc_state *old_crtc_state) > >> +{ > >> + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); > >> + struct drm_device *ddev = crtc->dev; > >> + struct tidss_device *tidss = ddev->dev_private; > >> + > >> + dev_dbg(ddev->dev, "%s, crtc enabled %d, event %p\n", > >> + __func__, tcrtc->enabled, crtc->state->event); > >> + > >> + /* Only flush the CRTC if it is currently enabled. */ > >> + if (!tcrtc->enabled) > > > > In atomic drivers state should be stored in state structures. You could > > check old_crtc_state for this and remove the enabled field in struct > > tidss_crtc. > > The variable is need for tracking the HW state trough the state > transition. I do not know which state variable I should use to keep that > state information stored trough the process where one state changes into > another. > > The drm_crtc_state already contains couple of variables describing > whether crtc is enabled or not, or if the mode is going to change in the > state transition (giving a hint that crtc is going go through > disable-enable cycle). I tried to use all of those, and the old state > variable, to accomplish the same behaviour as the current code has, but > I could not. > > One of the problematic cases was a new drm client making an atomic > commit, the old one being bf-console, with the same mode as the one was > using. In that situation the crtc goes trough disable-enable cycle, but > I could not find any way to detect the situation from the old and new > crtc state. Enable-disable cycle means that we should not flip the > go-bit, but just configure everything and enable the crtc, e.g skip the > atomic flush and wait for enable instead. Thanks for the report. If we can't detect this from the drm_crtc_state, I think it's a shortcoming of the KMS core, and should be fixed. Daniel, what's your opinion ? > In any case this is for HW state, not for DRM state tracking. I could > add a call back to dispc ops for asking if the video port is enabled and > use that instead if you think that is more formally correct. I don't think a callback is worth it. The idea behind drm_crtc_state, if I understand it correctly, is to track all state, software and hardware. One option could be to extent drm_crtc_state (as in subclassing the object) with a custom hardware enable field, but I thought this would be covered by the standard fields.
On Wed, Nov 7, 2018 at 2:40 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Jyri, > > (CC'ing Daniel Vetter) > > On Wednesday, 31 October 2018 18:24:28 EET Jyri Sarha wrote: > > Hi Laurent, > > Tomi is busy with other things so I have taken over applying these > > comments. The rest is more or less clear, or commented by Tomi, but this > > is something have not addressed: > > > > On 30/07/18 17:12, Laurent Pinchart wrote: > > >> +static void tidss_crtc_atomic_flush(struct drm_crtc *crtc, > > >> + struct drm_crtc_state *old_crtc_state) > > >> +{ > > >> + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); > > >> + struct drm_device *ddev = crtc->dev; > > >> + struct tidss_device *tidss = ddev->dev_private; > > >> + > > >> + dev_dbg(ddev->dev, "%s, crtc enabled %d, event %p\n", > > >> + __func__, tcrtc->enabled, crtc->state->event); > > >> + > > >> + /* Only flush the CRTC if it is currently enabled. */ > > >> + if (!tcrtc->enabled) > > > > > > In atomic drivers state should be stored in state structures. You could > > > check old_crtc_state for this and remove the enabled field in struct > > > tidss_crtc. > > > > The variable is need for tracking the HW state trough the state > > transition. I do not know which state variable I should use to keep that > > state information stored trough the process where one state changes into > > another. > > > > The drm_crtc_state already contains couple of variables describing > > whether crtc is enabled or not, or if the mode is going to change in the > > state transition (giving a hint that crtc is going go through > > disable-enable cycle). I tried to use all of those, and the old state > > variable, to accomplish the same behaviour as the current code has, but > > I could not. > > > > One of the problematic cases was a new drm client making an atomic > > commit, the old one being bf-console, with the same mode as the one was > > using. In that situation the crtc goes trough disable-enable cycle, but > > I could not find any way to detect the situation from the old and new > > crtc state. Enable-disable cycle means that we should not flip the > > go-bit, but just configure everything and enable the crtc, e.g skip the > > atomic flush and wait for enable instead. > > Thanks for the report. If we can't detect this from the drm_crtc_state, I > think it's a shortcoming of the KMS core, and should be fixed. Daniel, what's > your opinion ? drm_atomic_crtc_needs_modeset() not good enough? Also please nota that using atomic helpers when they don't fit your hw's need is very much discouraged. They do take care of some of the boring book-keeping for common cases. But eventually all big drivers end up writing partially their own frameworks, only picking the pieces from the shared atomic helpers that do fully fit. If you want to know how, check out how the helpers decide when to call the ->enable and ->disable hooks. In any case, fully agreed with Laurent, don't do this. -Daniel > > In any case this is for HW state, not for DRM state tracking. I could > > add a call back to dispc ops for asking if the video port is enabled and > > use that instead if you think that is more formally correct. > > I don't think a callback is worth it. The idea behind drm_crtc_state, if I > understand it correctly, is to track all state, software and hardware. One > option could be to extent drm_crtc_state (as in subclassing the object) with a > custom hardware enable field, but I thought this would be covered by the > standard fields. > > -- > Regards, > > Laurent Pinchart > > >
On Wed, Nov 7, 2018 at 3:10 PM Daniel Vetter <daniel@ffwll.ch> wrote: > > On Wed, Nov 7, 2018 at 2:40 PM Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: > > > > Hi Jyri, > > > > (CC'ing Daniel Vetter) > > > > On Wednesday, 31 October 2018 18:24:28 EET Jyri Sarha wrote: > > > Hi Laurent, > > > Tomi is busy with other things so I have taken over applying these > > > comments. The rest is more or less clear, or commented by Tomi, but this > > > is something have not addressed: > > > > > > On 30/07/18 17:12, Laurent Pinchart wrote: > > > >> +static void tidss_crtc_atomic_flush(struct drm_crtc *crtc, > > > >> + struct drm_crtc_state *old_crtc_state) > > > >> +{ > > > >> + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); > > > >> + struct drm_device *ddev = crtc->dev; > > > >> + struct tidss_device *tidss = ddev->dev_private; > > > >> + > > > >> + dev_dbg(ddev->dev, "%s, crtc enabled %d, event %p\n", > > > >> + __func__, tcrtc->enabled, crtc->state->event); > > > >> + > > > >> + /* Only flush the CRTC if it is currently enabled. */ > > > >> + if (!tcrtc->enabled) > > > > > > > > In atomic drivers state should be stored in state structures. You could > > > > check old_crtc_state for this and remove the enabled field in struct > > > > tidss_crtc. > > > > > > The variable is need for tracking the HW state trough the state > > > transition. I do not know which state variable I should use to keep that > > > state information stored trough the process where one state changes into > > > another. > > > > > > The drm_crtc_state already contains couple of variables describing > > > whether crtc is enabled or not, or if the mode is going to change in the > > > state transition (giving a hint that crtc is going go through > > > disable-enable cycle). I tried to use all of those, and the old state > > > variable, to accomplish the same behaviour as the current code has, but > > > I could not. > > > > > > One of the problematic cases was a new drm client making an atomic > > > commit, the old one being bf-console, with the same mode as the one was > > > using. In that situation the crtc goes trough disable-enable cycle, but > > > I could not find any way to detect the situation from the old and new > > > crtc state. Enable-disable cycle means that we should not flip the > > > go-bit, but just configure everything and enable the crtc, e.g skip the > > > atomic flush and wait for enable instead. > > > > Thanks for the report. If we can't detect this from the drm_crtc_state, I > > think it's a shortcoming of the KMS core, and should be fixed. Daniel, what's > > your opinion ? > > drm_atomic_crtc_needs_modeset() not good enough? Also please nota that > using atomic helpers when they don't fit your hw's need is very much > discouraged. They do take care of some of the boring book-keeping for > common cases. But eventually all big drivers end up writing partially > their own frameworks, only picking the pieces from the shared atomic > helpers that do fully fit. If you want to know how, check out how the > helpers decide when to call the ->enable and ->disable hooks. Another option: Overwrite the top-level atomic_check function, compute when exactly you need to set the go bit and when not (you have both old and new state, this fully describes the transition). And then use that special derived state to decided at runtime whether to flush the go bit or not. -Daniel > In any case, fully agreed with Laurent, don't do this. > -Daniel > > > > In any case this is for HW state, not for DRM state tracking. I could > > > add a call back to dispc ops for asking if the video port is enabled and > > > use that instead if you think that is more formally correct. > > > > I don't think a callback is worth it. The idea behind drm_crtc_state, if I > > understand it correctly, is to track all state, software and hardware. One > > option could be to extent drm_crtc_state (as in subclassing the object) with a > > custom hardware enable field, but I thought this would be covered by the > > standard fields. > > > > -- > > Regards, > > > > Laurent Pinchart > > > > > > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index deeefa7a1773..b8b6c28a6800 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -289,6 +289,8 @@ source "drivers/gpu/drm/pl111/Kconfig" source "drivers/gpu/drm/tve200/Kconfig" +source "drivers/gpu/drm/tidss/Kconfig" + # Keep legacy drivers last menuconfig DRM_LEGACY diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 50093ff4479b..3aa032fbb11a 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -103,3 +103,4 @@ obj-$(CONFIG_DRM_MXSFB) += mxsfb/ obj-$(CONFIG_DRM_TINYDRM) += tinydrm/ obj-$(CONFIG_DRM_PL111) += pl111/ obj-$(CONFIG_DRM_TVE200) += tve200/ +obj-$(CONFIG_DRM_TIDSS) += tidss/ diff --git a/drivers/gpu/drm/tidss/Kconfig b/drivers/gpu/drm/tidss/Kconfig new file mode 100644 index 000000000000..818666db08a4 --- /dev/null +++ b/drivers/gpu/drm/tidss/Kconfig @@ -0,0 +1,10 @@ +config DRM_TIDSS + tristate "DRM Support for TI Keystone" + depends on DRM && OF + depends on ARM || ARM64 || COMPILE_TEST + select DRM_KMS_HELPER + select DRM_KMS_CMA_HELPER + select DRM_GEM_CMA_HELPER + select VIDEOMODE_HELPERS + help + TI Keystone diff --git a/drivers/gpu/drm/tidss/Makefile b/drivers/gpu/drm/tidss/Makefile new file mode 100644 index 000000000000..ee6b24db0441 --- /dev/null +++ b/drivers/gpu/drm/tidss/Makefile @@ -0,0 +1,11 @@ +# SPDX-License-Identifier: GPL-2.0 + +tidss-y := tidss_drv.o \ + tidss_kms.o \ + tidss_crtc.o \ + tidss_plane.o \ + tidss_encoder.o \ + tidss_dispc6.o \ + tidss_irq.o + +obj-$(CONFIG_DRM_TIDSS) += tidss.o diff --git a/drivers/gpu/drm/tidss/tidss_crtc.c b/drivers/gpu/drm/tidss/tidss_crtc.c new file mode 100644 index 000000000000..22c11f1e3318 --- /dev/null +++ b/drivers/gpu/drm/tidss/tidss_crtc.c @@ -0,0 +1,390 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> + */ + +#include <drm/drmP.h> +#include <drm/drm_atomic.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_cma_helper.h> +#include <drm/drm_gem_cma_helper.h> +#include <drm/drm_plane_helper.h> +#include <uapi/linux/media-bus-format.h> + +#include "tidss_crtc.h" +#include "tidss_dispc.h" +#include "tidss_drv.h" +#include "tidss_irq.h" + +/* ----------------------------------------------------------------------------- + * Page Flip + */ + +static void tidss_crtc_finish_page_flip(struct tidss_crtc *tcrtc) +{ + struct drm_device *ddev = tcrtc->crtc.dev; + struct tidss_device *tidss = ddev->dev_private; + struct drm_pending_vblank_event *event; + unsigned long flags; + bool busy; + + /* + * New settings are taken into use at VFP, and GO bit is cleared at + * the same time. This happens before the vertical blank interrupt. + * So there is a small change that the driver sets GO bit after VFP, but + * before vblank, and we have to check for that case here. + */ + busy = tidss->dispc_ops->vp_go_busy(tidss->dispc, tcrtc->hw_videoport); + if (busy) + return; + + spin_lock_irqsave(&ddev->event_lock, flags); + + event = tcrtc->event; + tcrtc->event = NULL; + + if (!event) { + spin_unlock_irqrestore(&ddev->event_lock, flags); + return; + } + + dev_dbg(ddev->dev, "%s\n", __func__); + + drm_crtc_send_vblank_event(&tcrtc->crtc, event); + + spin_unlock_irqrestore(&ddev->event_lock, flags); + + drm_crtc_vblank_put(&tcrtc->crtc); +} + +void tidss_crtc_vblank_irq(struct drm_crtc *crtc) +{ + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); + + drm_crtc_handle_vblank(crtc); + + tidss_crtc_finish_page_flip(tcrtc); +} + +void tidss_crtc_framedone_irq(struct drm_crtc *crtc) +{ + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); + + complete(&tcrtc->framedone_completion); +} + +void tidss_crtc_error_irq(struct drm_crtc *crtc, u64 irqstatus) +{ + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); + + dev_err_ratelimited(crtc->dev->dev, "CRTC%u SYNC LOST: (irq %llx)\n", + tcrtc->hw_videoport, irqstatus); +} + + +/* ----------------------------------------------------------------------------- + * CRTC Functions + */ + +static int tidss_crtc_atomic_check(struct drm_crtc *crtc, + struct drm_crtc_state *state) +{ + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); + struct drm_device *ddev = crtc->dev; + struct tidss_device *tidss = ddev->dev_private; + const struct drm_display_mode *mode = &state->adjusted_mode; + struct tidss_crtc_state *tcrtc_state = to_tidss_crtc_state(state); + int r; + + dev_dbg(ddev->dev, "%s\n", __func__); + + if (!state->enable) + return 0; + + r = tidss->dispc_ops->vp_check_config(tidss->dispc, tcrtc->hw_videoport, + mode, + tcrtc_state->bus_format, + tcrtc_state->bus_flags); + + if (r == 0) + return 0; + + dev_err(ddev->dev, "%s: failed (%ux%u pclk %u kHz)\n", + __func__, mode->hdisplay, mode->vdisplay, mode->clock); + + return r; +} + +static void tidss_crtc_atomic_begin(struct drm_crtc *crtc, + struct drm_crtc_state *old_crtc_state) +{ + struct drm_device *ddev = crtc->dev; + + dev_dbg(ddev->dev, "%s\n", __func__); +} + +static void tidss_crtc_atomic_flush(struct drm_crtc *crtc, + struct drm_crtc_state *old_crtc_state) +{ + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); + struct drm_device *ddev = crtc->dev; + struct tidss_device *tidss = ddev->dev_private; + + dev_dbg(ddev->dev, "%s, crtc enabled %d, event %p\n", + __func__, tcrtc->enabled, crtc->state->event); + + /* Only flush the CRTC if it is currently enabled. */ + if (!tcrtc->enabled) + return; + + WARN_ON(tidss->dispc_ops->vp_go_busy(tidss->dispc, + tcrtc->hw_videoport)); + + // I think we always need the event to signal flip done + WARN_ON(!crtc->state->event); + + if (crtc->state->color_mgmt_changed) { + struct drm_color_lut *lut = NULL; + uint length = 0; + + if (crtc->state->gamma_lut) { + lut = (struct drm_color_lut *) + crtc->state->gamma_lut->data; + length = crtc->state->gamma_lut->length / + sizeof(*lut); + } + tidss->dispc_ops->vp_set_gamma(tidss->dispc, + tcrtc->hw_videoport, + lut, length); + } + + WARN_ON(drm_crtc_vblank_get(crtc) != 0); + + spin_lock_irq(&ddev->event_lock); + tidss->dispc_ops->vp_go(tidss->dispc, tcrtc->hw_videoport); + + if (crtc->state->event) { + tcrtc->event = crtc->state->event; + crtc->state->event = NULL; + } + + spin_unlock_irq(&ddev->event_lock); +} + +static void tidss_crtc_atomic_enable(struct drm_crtc *crtc, + struct drm_crtc_state *old_state) +{ + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); + struct tidss_crtc_state *tcrtc_state = to_tidss_crtc_state(crtc->state); + struct drm_device *ddev = crtc->dev; + struct tidss_device *tidss = ddev->dev_private; + const struct drm_display_mode *mode = &crtc->state->adjusted_mode; + int r; + + dev_dbg(ddev->dev, "%s, event %p\n", __func__, crtc->state->event); + + tidss->dispc_ops->runtime_get(tidss->dispc); + + r = tidss->dispc_ops->vp_set_clk_rate(tidss->dispc, tcrtc->hw_videoport, + mode->clock * 1000); + WARN_ON(r); + + r = tidss->dispc_ops->vp_enable_clk(tidss->dispc, tcrtc->hw_videoport); + WARN_ON(r); + + /* Turn vertical blanking interrupt reporting on. */ + drm_crtc_vblank_on(crtc); + + tcrtc->enabled = true; + + tidss->dispc_ops->vp_enable(tidss->dispc, tcrtc->hw_videoport, + mode, + tcrtc_state->bus_format, + tcrtc_state->bus_flags); + + spin_lock_irq(&ddev->event_lock); + + if (crtc->state->event) { + drm_crtc_send_vblank_event(crtc, crtc->state->event); + crtc->state->event = NULL; + } + + spin_unlock_irq(&ddev->event_lock); +} + +static void tidss_crtc_atomic_disable(struct drm_crtc *crtc, + struct drm_crtc_state *old_state) +{ + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); + struct drm_device *ddev = crtc->dev; + struct tidss_device *tidss = ddev->dev_private; + + dev_dbg(ddev->dev, "%s, event %p\n", __func__, crtc->state->event); + + reinit_completion(&tcrtc->framedone_completion); + + tidss->dispc_ops->vp_disable(tidss->dispc, tcrtc->hw_videoport); + + if (!wait_for_completion_timeout(&tcrtc->framedone_completion, + msecs_to_jiffies(500))) + dev_err(tidss->dev, "Timeout waiting for framedone on crtc %d", + tcrtc->hw_videoport); + + spin_lock_irq(&ddev->event_lock); + if (crtc->state->event) { + drm_crtc_send_vblank_event(crtc, crtc->state->event); + crtc->state->event = NULL; + } + spin_unlock_irq(&ddev->event_lock); + + tcrtc->enabled = false; + + drm_crtc_vblank_off(crtc); + + tidss->dispc_ops->vp_disable_clk(tidss->dispc, tcrtc->hw_videoport); + + tidss->dispc_ops->runtime_put(tidss->dispc); + + spin_lock_irq(&crtc->dev->event_lock); + if (crtc->state->event) { + drm_crtc_send_vblank_event(crtc, crtc->state->event); + crtc->state->event = NULL; + } + spin_unlock_irq(&crtc->dev->event_lock); +} + +static +enum drm_mode_status tidss_crtc_mode_valid(struct drm_crtc *crtc, + const struct drm_display_mode *mode) +{ + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); + struct drm_device *ddev = crtc->dev; + struct tidss_device *tidss = ddev->dev_private; + + return tidss->dispc_ops->vp_check_mode(tidss->dispc, + tcrtc->hw_videoport, + mode); +} + +static const struct drm_crtc_helper_funcs crtc_helper_funcs = { + .atomic_check = tidss_crtc_atomic_check, + .atomic_begin = tidss_crtc_atomic_begin, + .atomic_flush = tidss_crtc_atomic_flush, + .atomic_enable = tidss_crtc_atomic_enable, + .atomic_disable = tidss_crtc_atomic_disable, + + .mode_valid = tidss_crtc_mode_valid, +}; + +static void tidss_crtc_reset(struct drm_crtc *crtc) +{ + if (crtc->state) + __drm_atomic_helper_crtc_destroy_state(crtc->state); + + kfree(crtc->state); + crtc->state = kzalloc(sizeof(struct tidss_crtc_state), GFP_KERNEL); + + if (crtc->state) + crtc->state->crtc = crtc; +} + +static struct drm_crtc_state * +tidss_crtc_duplicate_state(struct drm_crtc *crtc) +{ + struct tidss_crtc_state *state, *current_state; + + if (WARN_ON(!crtc->state)) + return NULL; + + current_state = to_tidss_crtc_state(crtc->state); + + state = kmalloc(sizeof(*state), GFP_KERNEL); + if (!state) + return NULL; + + __drm_atomic_helper_crtc_duplicate_state(crtc, &state->base); + + state->bus_format = current_state->bus_format; + state->bus_flags = current_state->bus_flags; + + return &state->base; +} + +static int tidss_crtc_enable_vblank(struct drm_crtc *crtc) +{ + struct drm_device *ddev = crtc->dev; + struct tidss_device *tidss = ddev->dev_private; + + dev_dbg(ddev->dev, "%s\n", __func__); + + tidss->dispc_ops->runtime_get(tidss->dispc); + + tidss_irq_enable_vblank(crtc); + + return 0; +} + +static void tidss_crtc_disable_vblank(struct drm_crtc *crtc) +{ + struct drm_device *ddev = crtc->dev; + struct tidss_device *tidss = ddev->dev_private; + + dev_dbg(ddev->dev, "%s\n", __func__); + + tidss_irq_disable_vblank(crtc); + + tidss->dispc_ops->runtime_put(tidss->dispc); +} + +static const struct drm_crtc_funcs crtc_funcs = { + .reset = tidss_crtc_reset, + .destroy = drm_crtc_cleanup, + .set_config = drm_atomic_helper_set_config, + .page_flip = drm_atomic_helper_page_flip, + .atomic_duplicate_state = tidss_crtc_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, + .enable_vblank = tidss_crtc_enable_vblank, + .disable_vblank = tidss_crtc_disable_vblank, +}; + +struct tidss_crtc *tidss_crtc_create(struct tidss_device *tidss, u32 hw_videoport, + struct drm_plane *primary, struct device_node *epnode) +{ + struct tidss_crtc *tcrtc; + struct drm_crtc *crtc; + int ret; + + tcrtc = devm_kzalloc(tidss->dev, sizeof(*tcrtc), GFP_KERNEL); + if (!tcrtc) + return ERR_PTR(-ENOMEM); + + tcrtc->hw_videoport = hw_videoport; + init_completion(&tcrtc->framedone_completion); + + crtc = &tcrtc->crtc; + + ret = drm_crtc_init_with_planes(tidss->ddev, crtc, primary, + NULL, &crtc_funcs, NULL); + if (ret < 0) + return ERR_PTR(ret); + + drm_crtc_helper_add(crtc, &crtc_helper_funcs); + + /* The dispc API adapts to what ever size we ask from in no + * matter what HW supports. X-server assumes 256 element gamma + * tables so lets use that. Size of HW gamma table can be + * extracted with dispc_vp_gamma_size(). If it returns 0 + * gamma table is not supprted. + */ + if (tidss->dispc_ops->vp_gamma_size(tidss->dispc, hw_videoport)) { + uint gamma_lut_size = 256; + + drm_crtc_enable_color_mgmt(crtc, 0, false, gamma_lut_size); + drm_mode_crtc_set_gamma_size(crtc, gamma_lut_size); + } + + return tcrtc; +} diff --git a/drivers/gpu/drm/tidss/tidss_crtc.h b/drivers/gpu/drm/tidss/tidss_crtc.h new file mode 100644 index 000000000000..9f32d81e55d9 --- /dev/null +++ b/drivers/gpu/drm/tidss/tidss_crtc.h @@ -0,0 +1,49 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> + */ + +#ifndef __TIDSS_CRTC_H__ +#define __TIDSS_CRTC_H__ + +#include <linux/wait.h> +#include <linux/completion.h> +#include <drm/drm_crtc.h> + +#include "tidss_dispc.h" + +#define to_tidss_crtc(c) container_of((c), struct tidss_crtc, crtc) + +struct tidss_crtc { + struct drm_crtc crtc; + + u32 hw_videoport; + + struct drm_pending_vblank_event *event; + + /* has crtc_atomic_enable been called? */ + bool enabled; + + struct completion framedone_completion; +}; + +#define to_tidss_crtc_state(x) container_of(x, struct tidss_crtc_state, base) + +struct tidss_crtc_state { + /* Must be first. */ + struct drm_crtc_state base; + + u32 bus_format; + u32 bus_flags; +}; + +struct tidss_crtc *tidss_crtc_create(struct tidss_device *tidss, u32 hw_videoport, + struct drm_plane *primary, struct device_node *epnode); + + +void tidss_crtc_vblank_irq(struct drm_crtc *crtc); +void tidss_crtc_framedone_irq(struct drm_crtc *crtc); +void tidss_crtc_error_irq(struct drm_crtc *crtc, u64 irqstatus); + +#endif diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h new file mode 100644 index 000000000000..38ee3d75404e --- /dev/null +++ b/drivers/gpu/drm/tidss/tidss_dispc.h @@ -0,0 +1,145 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> + */ + +#ifndef __TIDSS_DISPC_H__ +#define __TIDSS_DISPC_H__ + +struct tidss_device; +struct dispc_device; +struct drm_color_lut; + +#define DSS_MAX_CHANNELS 8 +#define DSS_MAX_PLANES 8 + +/* + * Based on the above 2 defines the bellow defines describe following + * u64 IRQ bits: + * + * bit group |dev |mrg0|mrg1|mrg2|mrg3|mrg4|mrg5|mrg6|mrg7|plane 0-7|<unused> | + * bit use |Dfou|FEOL|FEOL|FEOL|FEOL|FEOL|FEOL|FEOL|FEOL|UUUU|UUUU| | | | | | + * bit number|0-3 |4-7 |8-11| 12-35 | 36-43 | 44-63 | + * + * device bits: D = OCP error + * WB bits: f = frame done wb, o = wb buffer overflow, + * u = wb buffer uncomplete + * vp bits: F = frame done, E = vsync even, O = vsync odd, L = sync lost + * plane bits: U = fifo underflow + */ + +#define DSS_IRQ_DEVICE_OCP_ERR BIT_ULL(0) + +#define DSS_IRQ_DEVICE_FRAMEDONEWB BIT_ULL(1) +#define DSS_IRQ_DEVICE_WBBUFFEROVERFLOW BIT_ULL(2) +#define DSS_IRQ_DEVICE_WBUNCOMPLETEERROR BIT_ULL(3) +#define DSS_IRQ_DEVICE_WB_MASK GENMASK_ULL(3, 1) + +#define DSS_IRQ_VP_BIT_N(ch, bit) (4 + 4 * ch + bit) +#define DSS_IRQ_PLANE_BIT_N(plane, bit) \ + (DSS_IRQ_VP_BIT_N(DSS_MAX_CHANNELS, 0) + 1 * plane + bit) + +#define DSS_IRQ_VP_BIT(ch, bit) BIT_ULL(DSS_IRQ_VP_BIT_N(ch, bit)) +#define DSS_IRQ_PLANE_BIT(plane, bit) BIT_ULL(DSS_IRQ_PLANE_BIT_N(plane, bit)) + +#define DSS_IRQ_VP_MASK(ch) \ + GENMASK_ULL(DSS_IRQ_VP_BIT_N(ch, 3), DSS_IRQ_VP_BIT_N(ch, 0)) +#define DSS_IRQ_PLANE_MASK(plane) \ + GENMASK_ULL(DSS_IRQ_PLANE_BIT_N(plane, 0), DSS_IRQ_PLANE_BIT_N(plane, 0)) + +#define DSS_IRQ_VP_FRAME_DONE(ch) DSS_IRQ_VP_BIT(ch, 0) +#define DSS_IRQ_VP_VSYNC_EVEN(ch) DSS_IRQ_VP_BIT(ch, 1) +#define DSS_IRQ_VP_VSYNC_ODD(ch) DSS_IRQ_VP_BIT(ch, 2) +#define DSS_IRQ_VP_SYNC_LOST(ch) DSS_IRQ_VP_BIT(ch, 3) + +#define DSS_IRQ_PLANE_FIFO_UNDERFLOW(plane) DSS_IRQ_PLANE_BIT(plane, 0) + +struct tidss_vp_info { + u32 default_color; +}; + +struct tidss_plane_info { + dma_addr_t paddr; + dma_addr_t p_uv_addr; /* for NV12 format */ + u16 fb_width; + u16 width; + u16 height; + u32 fourcc; + + u16 pos_x; + u16 pos_y; + u16 out_width; /* if 0, out_width == width */ + u16 out_height; /* if 0, out_height == height */ + u8 global_alpha; + u8 pre_mult_alpha; + u8 zorder; +}; + +struct dispc_ops { + u64 (*read_and_clear_irqstatus)(struct dispc_device *dispc); + void (*write_irqenable)(struct dispc_device *dispc, u64 enable); + + int (*runtime_get)(struct dispc_device *dispc); + void (*runtime_put)(struct dispc_device *dispc); + + int (*get_num_planes)(struct dispc_device *dispc); + int (*get_num_vps)(struct dispc_device *dispc); + + const char *(*plane_name)(struct dispc_device *dispc, + u32 hw_plane); + const char *(*vp_name)(struct dispc_device *dispc, + u32 hw_videoport); + + u32 (*get_memory_bandwidth_limit)(struct dispc_device *dispc); + + void (*vp_enable)(struct dispc_device *dispc, u32 hw_videoport, + const struct drm_display_mode *mode, + u32 bus_fmt, u32 bus_flags); + + void (*vp_disable)(struct dispc_device *dispc, u32 hw_videoport); + + bool (*vp_go_busy)(struct dispc_device *dispc, + u32 hw_videoport); + void (*vp_go)(struct dispc_device *dispc, u32 hw_videoport); + + void (*vp_setup)(struct dispc_device *dispc, u32 hw_videoport, + const struct tidss_vp_info *info); + + enum drm_mode_status (*vp_check_mode)(struct dispc_device *dispc, + u32 hw_videoport, + const struct drm_display_mode *mode); + + int (*vp_check_config)(struct dispc_device *dispc, u32 hw_videoport, + const struct drm_display_mode *mode, + u32 bus_fmt, u32 bus_flags); + + u32 (*vp_gamma_size)(struct dispc_device *dispc, + u32 hw_videoport); + void (*vp_set_gamma)(struct dispc_device *dispc, + u32 hw_videoport, + const struct drm_color_lut *lut, + unsigned int length); + + int (*plane_enable)(struct dispc_device *dispc, u32 hw_plane, + bool enable); + int (*plane_setup)(struct dispc_device *dispc, u32 hw_plane, + const struct tidss_plane_info *oi, + u32 hw_videoport); + + int (*vp_set_clk_rate)(struct dispc_device *dispc, + u32 hw_videoport, unsigned long rate); + int (*vp_enable_clk)(struct dispc_device *dispc, u32 hw_videoport); + void (*vp_disable_clk)(struct dispc_device *dispc, u32 hw_videoport); + + int (*runtime_suspend)(struct dispc_device *dispc); + int (*runtime_resume)(struct dispc_device *dispc); + + void (*remove)(struct dispc_device *dispc); + + int (*modeset_init)(struct dispc_device *dispc); +}; + +int dispc6_init(struct tidss_device *tidss); + +#endif diff --git a/drivers/gpu/drm/tidss/tidss_dispc6.c b/drivers/gpu/drm/tidss/tidss_dispc6.c new file mode 100644 index 000000000000..7772d4484a6e --- /dev/null +++ b/drivers/gpu/drm/tidss/tidss_dispc6.c @@ -0,0 +1,1450 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> + */ + +#include <linux/clk.h> +#include <linux/component.h> +#include <linux/delay.h> +#include <linux/export.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/of.h> +#include <linux/of_graph.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> + +#include <drm/drm_fourcc.h> +#include <drm/drm_of.h> +#include <drm/drm_panel.h> + +#include "tidss_crtc.h" +#include "tidss_drv.h" +#include "tidss_encoder.h" +#include "tidss_plane.h" + +#include "tidss_dispc6.h" + +static const struct { + u32 fmt; + u32 port_width; +} dispc6_bus_formats[] = { + { MEDIA_BUS_FMT_RGB444_1X12, 12 }, + { MEDIA_BUS_FMT_RGB565_1X16, 16 }, + { MEDIA_BUS_FMT_RGB666_1X18, 18 }, + { MEDIA_BUS_FMT_RGB888_1X24, 24 }, +}; + +/* + * TRM gives bitfields as start:end, where start is the higher bit + * number. For example 7:0 + */ + +#define FLD_MASK(start, end) (((1 << ((start) - (end) + 1)) - 1) << (end)) +#define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end)) +#define FLD_GET(val, start, end) (((val) & FLD_MASK(start, end)) >> (end)) +#define FLD_MOD(orig, val, start, end) \ + (((orig) & ~FLD_MASK(start, end)) | FLD_VAL(val, start, end)) + + +#define REG_GET(dispc, idx, start, end) \ + FLD_GET(dispc6_read(dispc, idx), start, end) + +#define REG_FLD_MOD(dispc, idx, val, start, end) \ + dispc6_write(dispc, idx, FLD_MOD(dispc6_read(dispc, idx), val, start, end)) + +#define VID_REG_GET(dispc, plane, idx, start, end) \ + FLD_GET(dispc6_vid_read(dispc, plane, idx), start, end) + +#define VID_REG_FLD_MOD(dispc, plane, idx, val, start, end) \ + dispc6_vid_write(dispc, plane, idx, FLD_MOD(dispc6_vid_read(dispc, plane, idx), val, start, end)) + + +#define VP_REG_GET(dispc, vp, idx, start, end) \ + FLD_GET(dispc6_vp_read(dispc, vp, idx), start, end) + +#define VP_REG_FLD_MOD(dispc, vp, idx, val, start, end) \ + dispc6_vp_write(dispc, vp, idx, FLD_MOD(dispc6_vp_read(dispc, vp, idx), val, start, end)) + +struct dispc_features { + /* XXX should these come from the .dts? Min pclk is not feature of DSS IP */ + unsigned long min_pclk; + unsigned long max_pclk; +}; + +/* Note: 9MHz is a special allowed case, and is handled separately in the code */ +static const struct dispc_features k2g_dispc_feats = { + .min_pclk = 43750000, + .max_pclk = 150000000, +}; + +static const struct of_device_id dispc6_of_match[] = { + { .compatible = "ti,k2g-dss", .data = &k2g_dispc_feats, }, + {}, +}; + +struct dispc_device { + struct device *dev; + + void __iomem *base_cfg; + void __iomem *base_common; + void __iomem *base_vid1; + void __iomem *base_ovr1; + void __iomem *base_vp1; + + const struct dispc_features *feat; + + struct clk *fclk; + struct clk *vp_clk; + + bool is_enabled; + + u32 gamma_table[256]; + + struct tidss_device *tidss; +}; + +static void dispc6_write(struct dispc_device *dispc, u16 reg, u32 val) +{ + iowrite32(val, dispc->base_common + reg); +} + +static u32 dispc6_read(struct dispc_device *dispc, u16 reg) +{ + return ioread32(dispc->base_common + reg); +} + +static void dispc6_vid_write(struct dispc_device *dispc, + u32 hw_plane, u16 reg, u32 val) +{ + void __iomem *base = dispc->base_vid1; + + iowrite32(val, base + reg); +} + +static u32 dispc6_vid_read(struct dispc_device *dispc, + u32 hw_plane, u16 reg) +{ + void __iomem *base = dispc->base_vid1; + + return ioread32(base + reg); +} + +static void dispc6_ovr_write(struct dispc_device *dispc, + u32 hw_videoport, u16 reg, u32 val) +{ + void __iomem *base = dispc->base_ovr1; + + iowrite32(val, base + reg); +} + +__maybe_unused +static u32 dispc6_ovr_read(struct dispc_device *dispc, + u32 hw_videoport, u16 reg) +{ + void __iomem *base = dispc->base_ovr1; + + return ioread32(base + reg); +} + +static void dispc6_vp_write(struct dispc_device *dispc, + u32 hw_videoport, u16 reg, u32 val) +{ + void __iomem *base = dispc->base_vp1; + + iowrite32(val, base + reg); +} + +static u32 dispc6_vp_read(struct dispc_device *dispc, + u32 hw_videoport, u16 reg) +{ + void __iomem *base = dispc->base_vp1; + + return ioread32(base + reg); +} + +static int dispc6_runtime_get(struct dispc_device *dispc) +{ + int r; + + dev_dbg(dispc->dev, "dispc_runtime_get\n"); + + r = pm_runtime_get_sync(dispc->dev); + WARN_ON(r < 0); + return r < 0 ? r : 0; +} + +static void dispc6_runtime_put(struct dispc_device *dispc) +{ + int r; + + dev_dbg(dispc->dev, "dispc_runtime_put\n"); + + r = pm_runtime_put_sync(dispc->dev); + WARN_ON(r < 0); +} + +static u64 dispc6_vp_irq_from_raw(u32 stat) +{ + u32 vp = 0; + u64 vp_stat = 0; + + if (stat & BIT(0)) + vp_stat |= DSS_IRQ_VP_FRAME_DONE(vp); + if (stat & BIT(1)) + vp_stat |= DSS_IRQ_VP_VSYNC_EVEN(vp); + if (stat & BIT(2)) + vp_stat |= DSS_IRQ_VP_VSYNC_ODD(vp); + if (stat & BIT(4)) + vp_stat |= DSS_IRQ_VP_SYNC_LOST(vp); + + return vp_stat; +} + +static u32 dispc6_vp_irq_to_raw(u64 vpstat) +{ + u32 vp = 0; + u32 stat = 0; + + if (vpstat & DSS_IRQ_VP_FRAME_DONE(vp)) + stat |= BIT(0); + if (vpstat & DSS_IRQ_VP_VSYNC_EVEN(vp)) + stat |= BIT(1); + if (vpstat & DSS_IRQ_VP_VSYNC_ODD(vp)) + stat |= BIT(2); + if (vpstat & DSS_IRQ_VP_SYNC_LOST(vp)) + stat |= BIT(4); + + return stat; +} + +static u64 dispc6_vid_irq_from_raw(u32 stat) +{ + uint plane = 0; + u64 vid_stat = 0; + + if (stat & BIT(0)) + vid_stat |= DSS_IRQ_PLANE_FIFO_UNDERFLOW(plane); + + return vid_stat; +} + +static u32 dispc6_vid_irq_to_raw(u64 vidstat) +{ + uint plane = 0; + u32 stat = 0; + + if (vidstat & DSS_IRQ_PLANE_FIFO_UNDERFLOW(plane)) + stat |= BIT(0); + + return stat; +} + + +static u64 dispc6_vp_read_irqstatus(struct dispc_device *dispc, + u32 hw_videoport) +{ + u32 stat = dispc6_vp_read(dispc, hw_videoport, DISPC_VP_IRQSTATUS); + + return dispc6_vp_irq_from_raw(stat); +} + +static void dispc6_vp_write_irqstatus(struct dispc_device *dispc, + u32 hw_videoport, + u64 vpstat) +{ + u32 stat = dispc6_vp_irq_to_raw(vpstat); + + dispc6_vp_write(dispc, hw_videoport, DISPC_VP_IRQSTATUS, stat); +} + +static u64 dispc6_vid_read_irqstatus(struct dispc_device *dispc, + u32 hw_plane) +{ + u32 stat = dispc6_vid_read(dispc, hw_plane, DISPC_VID_IRQSTATUS); + + return dispc6_vid_irq_from_raw(stat); +} + +static void dispc6_vid_write_irqstatus(struct dispc_device *dispc, + u32 hw_plane, + u64 vidstat) +{ + u32 stat = dispc6_vid_irq_to_raw(vidstat); + + dispc6_vid_write(dispc, hw_plane, DISPC_VID_IRQSTATUS, stat); +} + + +static u64 dispc6_vp_read_irqenable(struct dispc_device *dispc, + u32 hw_videoport) +{ + u32 stat = dispc6_vp_read(dispc, hw_videoport, DISPC_VP_IRQENABLE); + + return dispc6_vp_irq_from_raw(stat); +} + +static void dispc6_vp_write_irqenable(struct dispc_device *dispc, + u32 hw_videoport, + u64 vpstat) +{ + u32 stat = dispc6_vp_irq_to_raw(vpstat); + + dispc6_vp_write(dispc, hw_videoport, DISPC_VP_IRQENABLE, stat); +} + +static u64 dispc6_vid_read_irqenable(struct dispc_device *dispc, + u32 hw_plane) +{ + u32 stat = dispc6_vid_read(dispc, hw_plane, DISPC_VID_IRQENABLE); + + return dispc6_vid_irq_from_raw(stat); +} + +static void dispc6_vid_write_irqenable(struct dispc_device *dispc, + u32 hw_plane, + u64 vidstat) +{ + u32 stat = dispc6_vid_irq_to_raw(vidstat); + + dispc6_vid_write(dispc, hw_plane, DISPC_VID_IRQENABLE, stat); +} + + +static void dispc6_clear_irqstatus(struct dispc_device *dispc, u64 mask) +{ + dispc6_vp_write_irqstatus(dispc, 0, mask); + dispc6_vid_write_irqstatus(dispc, 0, mask); +} + +static u64 dispc6_read_and_clear_irqstatus(struct dispc_device *dispc) +{ + u64 stat = 0; + + // always clear the top level irqstatus + dispc6_write(dispc, DISPC_IRQSTATUS, + dispc6_read(dispc, DISPC_IRQSTATUS)); + + stat |= dispc6_vp_read_irqstatus(dispc, 0); + stat |= dispc6_vid_read_irqstatus(dispc, 0); + + dispc6_clear_irqstatus(dispc, stat); + + return stat; +} + +static u64 dispc6_read_irqenable(struct dispc_device *dispc) +{ + u64 stat = 0; + + stat |= dispc6_vp_read_irqenable(dispc, 0); + stat |= dispc6_vid_read_irqenable(dispc, 0); + + return stat; +} + +static void dispc6_write_irqenable(struct dispc_device *dispc, u64 mask) +{ + u64 old_mask = dispc6_read_irqenable(dispc); + + /* clear the irqstatus for newly enabled irqs */ + dispc6_clear_irqstatus(dispc, (mask ^ old_mask) & mask); + + dispc6_vp_write_irqenable(dispc, 0, mask); + dispc6_vid_write_irqenable(dispc, 0, mask); + + dispc6_write(dispc, DISPC_IRQENABLE_SET, (1 << 0) | (1 << 7)); + + /* flush posted write */ + dispc6_read_irqenable(dispc); +} + +static void dispc6_set_num_datalines(struct dispc_device *dispc, + u32 hw_videoport, int num_lines) +{ + int v; + + switch (num_lines) { + case 12: + v = 0; break; + case 16: + v = 1; break; + case 18: + v = 2; break; + case 24: + v = 3; break; + case 30: + v = 4; break; + case 36: + v = 5; break; + default: + WARN_ON(1); + v = 3; + break; + } + + VP_REG_FLD_MOD(dispc, hw_videoport, DISPC_VP_CONTROL, v, 10, 8); +} + +static void dispc6_vp_enable(struct dispc_device *dispc, u32 hw_videoport, + const struct drm_display_mode *mode, + u32 bus_fmt, u32 bus_flags) +{ + bool align, onoff, rf, ieo, ipc, ihs, ivs; + int i; + u32 port_width; + u32 hsw, hfp, hbp, vsw, vfp, vbp; + + for (i = 0; i < ARRAY_SIZE(dispc6_bus_formats); ++i) { + if (dispc6_bus_formats[i].fmt != bus_fmt) + continue; + + port_width = dispc6_bus_formats[i].port_width; + break; + } + + if (WARN_ON(i == ARRAY_SIZE(dispc6_bus_formats))) + return; + + dispc6_set_num_datalines(dispc, hw_videoport, port_width); + + hfp = mode->hsync_start - mode->hdisplay; + hsw = mode->hsync_end - mode->hsync_start; + hbp = mode->htotal - mode->hsync_end; + + vfp = mode->vsync_start - mode->vdisplay; + vsw = mode->vsync_end - mode->vsync_start; + vbp = mode->vtotal - mode->vsync_end; + + dispc6_vp_write(dispc, hw_videoport, DISPC_VP_TIMING_H, + FLD_VAL(hsw - 1, 7, 0) | + FLD_VAL(hfp - 1, 19, 8) | + FLD_VAL(hbp - 1, 31, 20)); + + dispc6_vp_write(dispc, hw_videoport, DISPC_VP_TIMING_V, + FLD_VAL(vsw - 1, 7, 0) | + FLD_VAL(vfp, 19, 8) | + FLD_VAL(vbp, 31, 20)); + + if (mode->flags & DRM_MODE_FLAG_NVSYNC) + ivs = true; + else + ivs = false; + + if (mode->flags & DRM_MODE_FLAG_NHSYNC) + ihs = true; + else + ihs = false; + + if (bus_flags & DRM_BUS_FLAG_DE_LOW) + ieo = true; + else + ieo = false; + + if (bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE) + ipc = true; + else + ipc = false; + + /* always use the 'rf' setting */ + onoff = true; + + if (bus_flags & DRM_BUS_FLAG_SYNC_NEGEDGE) + rf = false; + else + rf = true; + + /* always use aligned syncs */ + align = true; + + dispc6_vp_write(dispc, hw_videoport, DISPC_VP_POL_FREQ, + FLD_VAL(align, 18, 18) | + FLD_VAL(onoff, 17, 17) | + FLD_VAL(rf, 16, 16) | + FLD_VAL(ieo, 15, 15) | + FLD_VAL(ipc, 14, 14) | + FLD_VAL(ihs, 13, 13) | + FLD_VAL(ivs, 12, 12)); + + dispc6_vp_write(dispc, hw_videoport, DISPC_VP_SIZE_SCREEN, + FLD_VAL(mode->hdisplay - 1, 11, 0) | + FLD_VAL(mode->vdisplay - 1, 27, 16)); + + VP_REG_FLD_MOD(dispc, hw_videoport, DISPC_VP_CONTROL, 1, 0, 0); +} + +static void dispc6_vp_disable(struct dispc_device *dispc, u32 hw_videoport) +{ + VP_REG_FLD_MOD(dispc, hw_videoport, DISPC_VP_CONTROL, 0, 0, 0); +} + +static bool dispc6_vp_go_busy(struct dispc_device *dispc, + u32 hw_videoport) +{ + return VP_REG_GET(dispc, hw_videoport, DISPC_VP_CONTROL, 5, 5); +} + +static void dispc6_vp_go(struct dispc_device *dispc, + u32 hw_videoport) +{ + VP_REG_FLD_MOD(dispc, hw_videoport, DISPC_VP_CONTROL, 1, 5, 5); +} + +static u16 c8_to_c12(u8 c8) +{ + u16 c12; + + c12 = c8 << 4; + + /* Replication logic: Copy c8 4 MSB to 4 LSB for full scale c12 */ + c12 = c8 >> 4; + + return c12; +} + +static u64 argb8888_to_argb12121212(u32 argb8888) +{ + u8 a, r, g, b; + u64 v; + + a = (argb8888 >> 24) & 0xff; + r = (argb8888 >> 16) & 0xff; + g = (argb8888 >> 8) & 0xff; + b = (argb8888 >> 0) & 0xff; + + v = ((u64)c8_to_c12(a) << 36) | ((u64)c8_to_c12(r) << 24) | + ((u64)c8_to_c12(g) << 12) | (u64)c8_to_c12(b); + + return v; +} + +static void dispc6_vp_setup(struct dispc_device *dispc, + u32 hw_videoport, + const struct tidss_vp_info *info) +{ + u64 v; + + v = argb8888_to_argb12121212(info->default_color); + + dispc6_ovr_write(dispc, 0, DISPC_OVR_DEFAULT_COLOR, v & 0xffffffff); + dispc6_ovr_write(dispc, 0, DISPC_OVR_DEFAULT_COLOR2, + (v >> 32) & 0xffff); +} + +static enum drm_mode_status dispc6_vp_check_mode(struct dispc_device *dispc, + u32 hw_videoport, + const struct drm_display_mode *mode) +{ + u32 hsw, hfp, hbp, vsw, vfp, vbp; + + /* special case for 9MHz */ + if (mode->clock * 1000 < dispc->feat->min_pclk && mode->clock != 9000) + return MODE_CLOCK_LOW; + + if (mode->clock * 1000 > dispc->feat->max_pclk) + return MODE_CLOCK_HIGH; + + if (mode->hdisplay > 4096) + return MODE_BAD; + + if (mode->vdisplay > 4096) + return MODE_BAD; + + /* TODO: add interlace support */ + if (mode->flags & DRM_MODE_FLAG_INTERLACE) + return MODE_NO_INTERLACE; + + hfp = mode->hsync_start - mode->hdisplay; + hsw = mode->hsync_end - mode->hsync_start; + hbp = mode->htotal - mode->hsync_end; + + vfp = mode->vsync_start - mode->vdisplay; + vsw = mode->vsync_end - mode->vsync_start; + vbp = mode->vtotal - mode->vsync_end; + + if (hsw < 1 || hsw > 256 || + hfp < 1 || hfp > 4096 || + hbp < 1 || hbp > 4096) + return MODE_BAD_HVALUE; + + if (vsw < 1 || vsw > 256 || + vfp < 0 || vfp > 4095 || + vbp < 0 || vbp > 4095) + return MODE_BAD_VVALUE; + + return MODE_OK; +} + +static int dispc6_vp_check_config(struct dispc_device *dispc, u32 hw_videoport, + const struct drm_display_mode *mode, + u32 bus_fmt, u32 bus_flags) +{ + enum drm_mode_status ok; + int i; + + ok = dispc6_vp_check_mode(dispc, hw_videoport, mode); + if (ok != MODE_OK) + return -EINVAL; + + + for (i = 0; i < ARRAY_SIZE(dispc6_bus_formats); ++i) { + if (dispc6_bus_formats[i].fmt == bus_fmt) + break; + } + + if (i == ARRAY_SIZE(dispc6_bus_formats)) + return -EINVAL; + + return 0; +} + +static int dispc6_vp_enable_clk(struct dispc_device *dispc, u32 hw_videoport) +{ + return clk_prepare_enable(dispc->vp_clk); +} + +static void dispc6_vp_disable_clk(struct dispc_device *dispc, + u32 hw_videoport) +{ + clk_disable_unprepare(dispc->vp_clk); +} + +static int dispc6_vp_set_clk_rate(struct dispc_device *dispc, + u32 hw_videoport, unsigned long rate) +{ + int r; + unsigned long new_rate; + + r = clk_set_rate(dispc->vp_clk, rate); + if (r) { + dev_err(dispc->dev, "Failed to set vp clk rate to %lu\n", + rate); + return r; + } + + new_rate = clk_get_rate(dispc->vp_clk); + + if (rate != new_rate) + dev_warn(dispc->dev, + "Failed to get exact pix clock %lu != %lu\n", + rate, new_rate); + + dev_dbg(dispc->dev, "New VP rate %lu Hz (requested %lu Hz)\n", + clk_get_rate(dispc->vp_clk), rate); + + return 0; +} + +/* CSC */ + +struct color_conv_coef { + int ry, rcb, rcr; + int gy, gcb, gcr; + int by, bcb, bcr; + int roffset, goffset, boffset; + bool full_range; +}; + +static void dispc6_vid_write_color_conv_coefs(struct dispc_device *dispc, + u32 hw_plane, + const struct color_conv_coef *ct) +{ +#define CVAL(x, y) (FLD_VAL(x, 26, 16) | FLD_VAL(y, 10, 0)) + + dispc6_vid_write(dispc, hw_plane, + DISPC_VID_CONV_COEF(0), CVAL(ct->rcr, ct->ry)); + dispc6_vid_write(dispc, hw_plane, + DISPC_VID_CONV_COEF(1), CVAL(ct->gy, ct->rcb)); + dispc6_vid_write(dispc, hw_plane, + DISPC_VID_CONV_COEF(2), CVAL(ct->gcb, ct->gcr)); + dispc6_vid_write(dispc, hw_plane, + DISPC_VID_CONV_COEF(3), CVAL(ct->bcr, ct->by)); + dispc6_vid_write(dispc, hw_plane, + DISPC_VID_CONV_COEF(4), CVAL(0, ct->bcb)); + + dispc6_vid_write(dispc, hw_plane, DISPC_VID_CONV_COEF(5), + FLD_VAL(ct->roffset, 15, 3) | + FLD_VAL(ct->goffset, 31, 19)); + dispc6_vid_write(dispc, hw_plane, DISPC_VID_CONV_COEF(6), + FLD_VAL(ct->boffset, 15, 3)); + + VID_REG_FLD_MOD(dispc, hw_plane, DISPC_VID_ATTRIBUTES, + ct->full_range, 11, 11); + +#undef CVAL +} + +static void dispc6_vid_csc_setup(struct dispc_device *dispc) +{ + /* YUV -> RGB, ITU-R BT.601, full range */ + const struct color_conv_coef coefs_yuv2rgb_bt601_full = { + 256, 0, 358, + 256, -88, -182, + 256, 452, 0, + 0, -2048, -2048, + true, + }; + + dispc6_vid_write_color_conv_coefs(dispc, 0, &coefs_yuv2rgb_bt601_full); +} + +static void dispc6_vid_csc_enable(struct dispc_device *dispc, + u32 hw_plane, bool enable) +{ + VID_REG_FLD_MOD(dispc, hw_plane, DISPC_VID_ATTRIBUTES, !!enable, 9, 9); +} + +/* SCALER */ + +static u32 dispc6_calc_fir_inc(u32 in, u32 out) +{ + return (u32)div_u64(0x200000ull * in, out); +} + +struct dispc6_vid_fir_coefs { + s16 c2[16]; + s16 c1[16]; + u16 c0[9]; +}; + +static const struct dispc6_vid_fir_coefs dispc6_fir_coefs_null = { + .c2 = { 0 }, + .c1 = { 0 }, + .c0 = { 512, 512, 512, 512, 512, 512, 512, 512, 256, }, +}; + +/* M=8, Upscale x >= 1 */ +static const struct dispc6_vid_fir_coefs dispc6_fir_coefs_m8 = { + .c2 = { 0, -4, -8, -16, -24, -32, -40, -48, 0, 2, 4, 6, 8, 6, 4, 2, }, + .c1 = { 0, 28, 56, 94, 132, 176, 220, 266, -56, -60, -64, -62, -60, -50, -40, -20, }, + .c0 = { 512, 506, 500, 478, 456, 424, 392, 352, 312, }, +}; + +/* 5-tap, M=22, Downscale Ratio 2.5 < x < 3 */ +static const struct dispc6_vid_fir_coefs dispc6_fir_coefs_m22_5tap = { + .c2 = { 16, 20, 24, 30, 36, 42, 48, 56, 0, 0, 0, 2, 4, 8, 12, 14, }, + .c1 = { 132, 140, 148, 156, 164, 172, 180, 186, 64, 72, 80, 88, 96, 104, 112, 122, }, + .c0 = { 216, 216, 216, 214, 212, 208, 204, 198, 192, }, +}; + +/* 3-tap, M=22, Downscale Ratio 2.5 < x < 3 */ +static const struct dispc6_vid_fir_coefs dispc6_fir_coefs_m22_3tap = { + .c1 = { 100, 118, 136, 156, 176, 196, 216, 236, 0, 10, 20, 30, 40, 54, 68, 84, }, + .c0 = { 312, 310, 308, 302, 296, 286, 276, 266, 256, }, +}; + +enum dispc6_vid_fir_coef_set { + DISPC6_VID_FIR_COEF_HORIZ, + DISPC6_VID_FIR_COEF_HORIZ_UV, + DISPC6_VID_FIR_COEF_VERT, + DISPC6_VID_FIR_COEF_VERT_UV, +}; + +static void dispc6_vid_write_fir_coefs(struct dispc_device *dispc, + u32 hw_plane, + enum dispc6_vid_fir_coef_set coef_set, + const struct dispc6_vid_fir_coefs *coefs) +{ + static const u16 c0_regs[] = { + [DISPC6_VID_FIR_COEF_HORIZ] = DISPC_VID_FIR_COEFS_H0, + [DISPC6_VID_FIR_COEF_HORIZ_UV] = DISPC_VID_FIR_COEFS_H0_C, + [DISPC6_VID_FIR_COEF_VERT] = DISPC_VID_FIR_COEFS_V0, + [DISPC6_VID_FIR_COEF_VERT_UV] = DISPC_VID_FIR_COEFS_V0_C, + }; + + static const u16 c12_regs[] = { + [DISPC6_VID_FIR_COEF_HORIZ] = DISPC_VID_FIR_COEFS_H12, + [DISPC6_VID_FIR_COEF_HORIZ_UV] = DISPC_VID_FIR_COEFS_H12_C, + [DISPC6_VID_FIR_COEF_VERT] = DISPC_VID_FIR_COEFS_V12, + [DISPC6_VID_FIR_COEF_VERT_UV] = DISPC_VID_FIR_COEFS_V12_C, + }; + + const u16 c0_base = c0_regs[coef_set]; + const u16 c12_base = c12_regs[coef_set]; + int phase; + + for (phase = 0; phase <= 8; ++phase) { + u16 reg = c0_base + phase * 4; + u16 c0 = coefs->c0[phase]; + + dispc6_vid_write(dispc, hw_plane, reg, c0); + } + + for (phase = 0; phase <= 15; ++phase) { + u16 reg = c12_base + phase * 4; + s16 c1, c2; + u32 c12; + + c1 = coefs->c1[phase]; + c2 = coefs->c2[phase]; + c12 = FLD_VAL(c1, 19, 10) | FLD_VAL(c2, 29, 20); + + dispc6_vid_write(dispc, hw_plane, reg, c12); + } +} + +static void dispc6_vid_write_scale_coefs(struct dispc_device *dispc, + u32 hw_plane) +{ + dispc6_vid_write_fir_coefs(dispc, hw_plane, DISPC6_VID_FIR_COEF_HORIZ, + &dispc6_fir_coefs_null); + dispc6_vid_write_fir_coefs(dispc, hw_plane, DISPC6_VID_FIR_COEF_HORIZ_UV, + &dispc6_fir_coefs_null); + dispc6_vid_write_fir_coefs(dispc, hw_plane, DISPC6_VID_FIR_COEF_VERT, + &dispc6_fir_coefs_null); + dispc6_vid_write_fir_coefs(dispc, hw_plane, DISPC6_VID_FIR_COEF_VERT_UV, + &dispc6_fir_coefs_null); +} + +static void dispc6_vid_set_scaling(struct dispc_device *dispc, + u32 hw_plane, + u32 orig_width, u32 orig_height, + u32 out_width, u32 out_height, + u32 fourcc) +{ + u32 in_w, in_h, in_w_uv, in_h_uv; + u32 fir_hinc, fir_vinc, fir_hinc_uv, fir_vinc_uv; + bool scale_x, scale_y; + bool five_taps = false; /* XXX always 3-tap for now */ + + in_w = in_w_uv = orig_width; + in_h = in_h_uv = orig_height; + + switch (fourcc) { + case DRM_FORMAT_NV12: + /* UV is subsampled by 2 horizontally and vertically */ + in_h_uv >>= 1; + in_w_uv >>= 1; + break; + + case DRM_FORMAT_YUYV: + case DRM_FORMAT_UYVY: + /* UV is subsampled by 2 horizontally */ + in_w_uv >>= 1; + break; + + default: + break; + } + + scale_x = in_w != out_width || in_w_uv != out_width; + scale_y = in_h != out_height || in_h_uv != out_height; + + /* HORIZONTAL RESIZE ENABLE */ + VID_REG_FLD_MOD(dispc, hw_plane, DISPC_VID_ATTRIBUTES, scale_x, 7, 7); + + /* VERTICAL RESIZE ENABLE */ + VID_REG_FLD_MOD(dispc, hw_plane, DISPC_VID_ATTRIBUTES, scale_y, 8, 8); + + /* Skip the rest if no scaling is used */ + if (!scale_x && !scale_y) + return; + + /* VERTICAL 5-TAPS */ + VID_REG_FLD_MOD(dispc, hw_plane, DISPC_VID_ATTRIBUTES, five_taps, 21, 21); + + /* FIR INC */ + + fir_hinc = dispc6_calc_fir_inc(in_w, out_width); + fir_vinc = dispc6_calc_fir_inc(in_h, out_height); + fir_hinc_uv = dispc6_calc_fir_inc(in_w_uv, out_width); + fir_vinc_uv = dispc6_calc_fir_inc(in_h_uv, out_height); + + dispc6_vid_write(dispc, hw_plane, DISPC_VID_FIRH, fir_hinc); + dispc6_vid_write(dispc, hw_plane, DISPC_VID_FIRV, fir_vinc); + dispc6_vid_write(dispc, hw_plane, DISPC_VID_FIRH2, fir_hinc_uv); + dispc6_vid_write(dispc, hw_plane, DISPC_VID_FIRV2, fir_vinc_uv); + + dispc6_vid_write_scale_coefs(dispc, hw_plane); +} + +/* OTHER */ + +static const struct { + u32 fourcc; + u8 dss_code; + u8 bytespp; +} dispc6_color_formats[] = { + { DRM_FORMAT_ARGB4444, 0x0, 2, }, + { DRM_FORMAT_ABGR4444, 0x1, 2, }, + { DRM_FORMAT_RGBA4444, 0x2, 2, }, + + { DRM_FORMAT_RGB565, 0x3, 2, }, + { DRM_FORMAT_BGR565, 0x4, 2, }, + + { DRM_FORMAT_ARGB1555, 0x5, 2, }, + { DRM_FORMAT_ABGR1555, 0x6, 2, }, + + { DRM_FORMAT_ARGB8888, 0x7, 4, }, + { DRM_FORMAT_ABGR8888, 0x8, 4, }, + { DRM_FORMAT_RGBA8888, 0x9, 4, }, + { DRM_FORMAT_BGRA8888, 0xa, 4, }, + + { DRM_FORMAT_XRGB8888, 0x27, 4, }, + { DRM_FORMAT_XBGR8888, 0x28, 4, }, + { DRM_FORMAT_RGBX8888, 0x29, 4, }, + { DRM_FORMAT_BGRX8888, 0x2a, 4, }, + + { DRM_FORMAT_YUYV, 0x3e, 2, }, + { DRM_FORMAT_UYVY, 0x3f, 2, }, + + { DRM_FORMAT_NV12, 0x3d, 2, }, +}; + +static bool dispc6_fourcc_is_yuv(u32 fourcc) +{ + switch (fourcc) { + case DRM_FORMAT_YUYV: + case DRM_FORMAT_UYVY: + case DRM_FORMAT_NV12: + return true; + default: + return false; + } +} + +static void dispc6_plane_set_pixel_format(struct dispc_device *dispc, + u32 hw_plane, u32 fourcc) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(dispc6_color_formats); ++i) { + if (dispc6_color_formats[i].fourcc == fourcc) { + VID_REG_FLD_MOD(dispc, hw_plane, DISPC_VID_ATTRIBUTES, + dispc6_color_formats[i].dss_code, + 6, 1); + return; + } + } + + WARN_ON(1); +} + +static int dispc6_fourcc_to_bytespp(u32 fourcc) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(dispc6_color_formats); ++i) { + if (dispc6_color_formats[i].fourcc == fourcc) + return dispc6_color_formats[i].bytespp; + } + + WARN_ON(1); + return 4; +} + +static s32 pixinc(int pixels, u8 ps) +{ + if (pixels == 1) + return 1; + else if (pixels > 1) + return 1 + (pixels - 1) * ps; + else if (pixels < 0) + return 1 - (-pixels + 1) * ps; + + WARN_ON(1); + return 0; +} + +static int dispc6_plane_setup(struct dispc_device *dispc, u32 hw_plane, + const struct tidss_plane_info *oi, + u32 hw_videoport) +{ + u32 fourcc = oi->fourcc; + int bytespp = dispc6_fourcc_to_bytespp(fourcc); + + dispc6_plane_set_pixel_format(dispc, hw_plane, fourcc); + + dispc6_vid_write(dispc, hw_plane, DISPC_VID_BA_0, oi->paddr); + dispc6_vid_write(dispc, hw_plane, DISPC_VID_BA_1, oi->paddr); + + dispc6_vid_write(dispc, hw_plane, DISPC_VID_BA_UV_0, oi->p_uv_addr); + dispc6_vid_write(dispc, hw_plane, DISPC_VID_BA_UV_1, oi->p_uv_addr); + + dispc6_vid_write(dispc, hw_plane, DISPC_VID_PICTURE_SIZE, + (oi->width - 1) | ((oi->height - 1) << 16)); + + dispc6_vid_write(dispc, hw_plane, DISPC_VID_PIXEL_INC, + pixinc(1, bytespp)); + dispc6_vid_write(dispc, hw_plane, DISPC_VID_ROW_INC, + pixinc(1 + oi->fb_width - oi->width, bytespp)); + + dispc6_vid_write(dispc, hw_plane, DISPC_VID_POSITION, + oi->pos_x | (oi->pos_y << 16)); + + dispc6_vid_write(dispc, hw_plane, DISPC_VID_SIZE, + (oi->out_width - 1) | ((oi->out_height - 1) << 16)); + + dispc6_vid_set_scaling(dispc, hw_plane, + oi->width, oi->height, + oi->out_width, oi->out_height, + fourcc); + + /* enable YUV->RGB color conversion */ + if (dispc6_fourcc_is_yuv(fourcc)) + dispc6_vid_csc_enable(dispc, hw_plane, true); + else + dispc6_vid_csc_enable(dispc, hw_plane, false); + + /* hw_videoport */ + VID_REG_FLD_MOD(dispc, hw_plane, DISPC_VID_ATTRIBUTES, 0, 16, 14); + + return 0; +} + +static int dispc6_plane_enable(struct dispc_device *dispc, + u32 hw_plane, bool enable) +{ + VID_REG_FLD_MOD(dispc, hw_plane, DISPC_VID_ATTRIBUTES, !!enable, 0, 0); + return 0; +} + +static u32 dispc6_vid_get_fifo_size(struct dispc_device *dispc, + u32 hw_plane) +{ + const u32 unit_size = 16; /* 128-bits */ + + return VID_REG_GET(dispc, hw_plane, DISPC_VID_BUF_SIZE_STATUS, 15, 0) * + unit_size; +} + +static void dispc6_vid_set_mflag_threshold(struct dispc_device *dispc, + u32 hw_plane, + u32 low, u32 high) +{ + dispc6_vid_write(dispc, hw_plane, DISPC_VID_MFLAG_THRESHOLD, + FLD_VAL(high, 31, 16) | FLD_VAL(low, 15, 0)); +} + +static void dispc6_mflag_setup(struct dispc_device *dispc) +{ + u32 hw_plane = 0; + const u32 unit_size = 16; /* 128-bits */ + u32 size = dispc6_vid_get_fifo_size(dispc, hw_plane); + u32 low, high; + + /* MFLAG_CTRL */ + REG_FLD_MOD(dispc, DISPC_GLOBAL_MFLAG_ATTRIBUTE, 1, 1, 0); + /* MFLAG_START */ + REG_FLD_MOD(dispc, DISPC_GLOBAL_MFLAG_ATTRIBUTE, 0, 2, 2); + + /* + * Simulation team suggests below thesholds: + * HT = fifosize * 5 / 8; + * LT = fifosize * 4 / 8; + */ + + low = size * 4 / 8 / unit_size; + high = size * 5 / 8 / unit_size; + + dispc6_vid_set_mflag_threshold(dispc, hw_plane, low, high); +} + +static void dispc6_initial_config(struct dispc_device *dispc) +{ + dispc6_vid_csc_setup(dispc); + dispc6_mflag_setup(dispc); + + /* Enable the gamma Shadow bit-field */ + VP_REG_FLD_MOD(dispc, 0, DISPC_VP_CONFIG, 1, 2, 2); +} + +static int dispc6_init_features(struct dispc_device *dispc) +{ + const struct of_device_id *match; + + match = of_match_node(dispc6_of_match, dispc->dev->of_node); + if (!match) { + dev_err(dispc->dev, "Unsupported DISPC version\n"); + return -ENODEV; + } + + dispc->feat = match->data; + + return 0; +} + +static int dispc6_get_num_planes(struct dispc_device *dispc) +{ + return 1; +} + +static int dispc6_get_num_vps(struct dispc_device *dispc) +{ + return 1; +} + +static u32 dispc6_vp_gamma_size(struct dispc_device *dispc, + u32 hw_videoport) +{ + return ARRAY_SIZE(dispc->gamma_table); +} + +static void dispc6_vp_write_gamma_table(struct dispc_device *dispc, + u32 hw_videoport) +{ + u32 *table = dispc->gamma_table; + uint hwlen = ARRAY_SIZE(dispc->gamma_table); + unsigned int i; + + dev_dbg(dispc->dev, "%s: hw_videoport %d\n", __func__, hw_videoport); + + for (i = 0; i < hwlen; ++i) { + u32 v = table[i]; + + v |= i << 24; + + dispc6_vp_write(dispc, hw_videoport, DISPC_VP_GAMMA_TABLE, v); + } +} + +static void dispc6_restore_gamma_tables(struct dispc_device *dispc) +{ + dev_dbg(dispc->dev, "%s()\n", __func__); + + dispc6_vp_write_gamma_table(dispc, 0); +} + +static const struct drm_color_lut dispc6_vp_gamma_default_lut[] = { + { .red = 0, .green = 0, .blue = 0, }, + { .red = U16_MAX, .green = U16_MAX, .blue = U16_MAX, }, +}; + +static void dispc6_vp_set_gamma(struct dispc_device *dispc, + u32 hw_videoport, + const struct drm_color_lut *lut, + unsigned int length) +{ + u32 *table = dispc->gamma_table; + uint hwlen = ARRAY_SIZE(dispc->gamma_table); + static const uint hwbits = 8; + uint i; + + dev_dbg(dispc->dev, "%s: hw_videoport %d, lut len %u, hw len %u\n", + __func__, hw_videoport, length, hwlen); + + if (lut == NULL || length < 2) { + lut = dispc6_vp_gamma_default_lut; + length = ARRAY_SIZE(dispc6_vp_gamma_default_lut); + } + + for (i = 0; i < length - 1; ++i) { + uint first = i * (hwlen - 1) / (length - 1); + uint last = (i + 1) * (hwlen - 1) / (length - 1); + uint w = last - first; + u16 r, g, b; + uint j; + + if (w == 0) + continue; + + for (j = 0; j <= w; j++) { + r = (lut[i].red * (w - j) + lut[i + 1].red * j) / w; + g = (lut[i].green * (w - j) + lut[i + 1].green * j) / w; + b = (lut[i].blue * (w - j) + lut[i + 1].blue * j) / w; + + r >>= 16 - hwbits; + g >>= 16 - hwbits; + b >>= 16 - hwbits; + + table[first + j] = (r << (hwbits * 2)) | + (g << hwbits) | b; + } + } + + if (dispc->is_enabled) + dispc6_vp_write_gamma_table(dispc, hw_videoport); +} + +static int dispc6_init_gamma_tables(struct dispc_device *dispc) +{ + dispc6_vp_set_gamma(dispc, 0, NULL, 0); + + return 0; +} + +static u32 dispc6_get_memory_bandwidth_limit(struct dispc_device *dispc) +{ + u32 limit = 0; + + /* Optional maximum memory bandwidth */ + of_property_read_u32(dispc->dev->of_node, "max-memory-bandwidth", + &limit); + + return limit; +} + +static const char *dispc6_plane_name(struct dispc_device *dispc, + u32 hw_plane) +{ + return "vid1"; +} + +static const char *dispc6_vp_name(struct dispc_device *dispc, + u32 hw_videoport) +{ + return "vp1"; +} + +static int dispc6_runtime_suspend(struct dispc_device *dispc) +{ + struct device *dev = dispc->dev; + + dev_dbg(dev, "suspend\n"); + + dispc->is_enabled = false; + + clk_disable_unprepare(dispc->fclk); + + return 0; +} + +static int dispc6_runtime_resume(struct dispc_device *dispc) +{ + struct device *dev = dispc->dev; + + dev_dbg(dev, "resume\n"); + + clk_prepare_enable(dispc->fclk); + + if (REG_GET(dispc, DISPC_SYSSTATUS, 0, 0) == 0) + dev_warn(dev, "DISPC FUNC RESET not done!\n"); + if (REG_GET(dispc, DISPC_SYSSTATUS, 1, 1) == 0) + dev_warn(dev, "DISPC VP RESET not done!\n"); + + dispc6_initial_config(dispc); + + dispc6_restore_gamma_tables(dispc); + + dispc->is_enabled = true; + + return 0; +} + +static int dispc6_modeset_init(struct dispc_device *dispc) +{ + struct tidss_device *tidss = dispc->tidss; + struct device *dev = tidss->dev; + const u32 hw_videoport = 0; + const u32 crtc_mask = 1; + const u32 hw_plane_id = 0; + struct drm_panel *panel; + struct drm_bridge *bridge; + u32 enc_type; + int ret; + struct tidss_plane *tplane; + struct tidss_crtc *tcrtc; + struct tidss_encoder *tenc; + struct device_node *epnode; + u32 fourccs[ARRAY_SIZE(dispc6_color_formats)]; + int i; + + /* first find if there is a connected panel/bridge */ + + ret = drm_of_find_panel_or_bridge(dev->of_node, hw_videoport, 0, &panel, &bridge); + if (ret) { + dev_dbg(dev, "no panel or bridge found\n"); + return ret; + } + + epnode = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0); + if (WARN_ON(!epnode)) + return -EINVAL; + + if (panel) { + dev_dbg(dev, "Setting up panel\n"); + + enc_type = DRM_MODE_ENCODER_DPI; + + bridge = devm_drm_panel_bridge_add(dev, panel, DRM_MODE_CONNECTOR_DPI); + if (IS_ERR(bridge)) { + dev_err(dev, "failed to set up panel bridge\n"); + return PTR_ERR(bridge); + } + } else { + enc_type = DRM_MODE_ENCODER_NONE; + } + + /* then create a plane, a crtc and an encoder for the panel/bridge */ + + for (i = 0; i < ARRAY_SIZE(dispc6_color_formats); ++i) + fourccs[i] = dispc6_color_formats[i].fourcc; + + tplane = tidss_plane_create(tidss, hw_plane_id, DRM_PLANE_TYPE_PRIMARY, + crtc_mask, fourccs, ARRAY_SIZE(fourccs)); + if (IS_ERR(tplane)) { + dev_err(tidss->dev, "plane create failed\n"); + return PTR_ERR(tplane); + } + + tidss->planes[tidss->num_planes++] = &tplane->plane; + + tcrtc = tidss_crtc_create(tidss, hw_videoport, &tplane->plane, epnode); + if (IS_ERR(tcrtc)) { + dev_err(tidss->dev, "crtc create failed\n"); + return PTR_ERR(tcrtc); + } + + tidss->crtcs[tidss->num_crtcs++] = &tcrtc->crtc; + + tenc = tidss_encoder_create(tidss, enc_type, 1 << tcrtc->crtc.index); + if (IS_ERR(tenc)) { + dev_err(tidss->dev, "encoder create failed\n"); + return PTR_ERR(tenc); + } + + ret = drm_bridge_attach(&tenc->encoder, bridge, NULL); + if (ret) { + dev_err(tidss->dev, "bridge attach failed: %d\n", ret); + return ret; + } + + return 0; +} + +static void dispc6_remove(struct dispc_device *dispc); + +static const struct dispc_ops dispc6_ops = { + .read_and_clear_irqstatus = dispc6_read_and_clear_irqstatus, + .write_irqenable = dispc6_write_irqenable, + + .runtime_get = dispc6_runtime_get, + .runtime_put = dispc6_runtime_put, + + .get_num_planes = dispc6_get_num_planes, + .get_num_vps = dispc6_get_num_vps, + + .plane_name = dispc6_plane_name, + .vp_name = dispc6_vp_name, + + .get_memory_bandwidth_limit = dispc6_get_memory_bandwidth_limit, + + .vp_enable = dispc6_vp_enable, + .vp_disable = dispc6_vp_disable, + .vp_go_busy = dispc6_vp_go_busy, + .vp_go = dispc6_vp_go, + + .vp_setup = dispc6_vp_setup, + .vp_check_mode = dispc6_vp_check_mode, + .vp_check_config = dispc6_vp_check_config, + + .vp_gamma_size = dispc6_vp_gamma_size, + .vp_set_gamma = dispc6_vp_set_gamma, + + .plane_enable = dispc6_plane_enable, + .plane_setup = dispc6_plane_setup, + + .vp_set_clk_rate = dispc6_vp_set_clk_rate, + .vp_enable_clk = dispc6_vp_enable_clk, + .vp_disable_clk = dispc6_vp_disable_clk, + + .runtime_suspend = dispc6_runtime_suspend, + .runtime_resume = dispc6_runtime_resume, + + .remove = dispc6_remove, + + .modeset_init = dispc6_modeset_init, +}; + +static int dispc6_iomap_resource(struct platform_device *pdev, const char *name, + void __iomem **base) +{ + struct resource *res; + void __iomem *b; + + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name); + b = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(b)) { + dev_err(&pdev->dev, "cannot ioremap resource '%s'\n", name); + return PTR_ERR(b); + } + + *base = b; + + return 0; +} + +int dispc6_init(struct tidss_device *tidss) +{ + struct device *dev = tidss->dev; + struct platform_device *pdev = tidss->pdev; + struct dispc_device *dispc; + int r; + + dev_dbg(dev, "%s\n", __func__); + + dispc = devm_kzalloc(dev, sizeof(*dispc), GFP_KERNEL); + if (!dispc) + return -ENOMEM; + + dispc->tidss = tidss; + dispc->dev = dev; + + r = dispc6_init_features(dispc); + if (r) + goto err_free; + + r = dispc6_iomap_resource(pdev, "cfg", &dispc->base_cfg); + if (r) + goto err_free; + + r = dispc6_iomap_resource(pdev, "common", &dispc->base_common); + if (r) + goto err_free; + + r = dispc6_iomap_resource(pdev, "vid1", &dispc->base_vid1); + if (r) + goto err_free; + + r = dispc6_iomap_resource(pdev, "ovr1", &dispc->base_ovr1); + if (r) + goto err_free; + + r = dispc6_iomap_resource(pdev, "vp1", &dispc->base_vp1); + if (r) + goto err_free; + + dev_dbg(dev, "dispc6_bind: iores ok\n"); + + dispc->fclk = devm_clk_get(dev, "fck"); + if (IS_ERR(dispc->fclk)) { + dev_err(dev, "Failed to get fclk\n"); + r = PTR_ERR(dispc->fclk); + goto err_free; + } + + dispc->vp_clk = devm_clk_get(dev, "vp1"); + if (IS_ERR(dispc->vp_clk)) { + dev_err(dev, "Failed to get vp1 clk\n"); + r = PTR_ERR(dispc->vp_clk); + goto err_free; + } + + r = dispc6_init_gamma_tables(dispc); + if (r) + goto err_free; + + tidss->dispc_ops = &dispc6_ops; + tidss->dispc = dispc; + + dev_dbg(dev, "%s done\n", __func__); + + return 0; +err_free: + dev_err(dev, "%s failed: %d\n", __func__, r); + return r; +} + +static void dispc6_remove(struct dispc_device *dispc) +{ + struct device *dev = dispc->dev; + + dev_dbg(dev, "dispc6_unbind\n"); + + dispc->tidss->dispc_ops = NULL; + dispc->tidss->dispc = NULL; + + dev_dbg(dev, "dispc6_unbind done\n"); +} diff --git a/drivers/gpu/drm/tidss/tidss_dispc6.h b/drivers/gpu/drm/tidss/tidss_dispc6.h new file mode 100644 index 000000000000..80197c812acd --- /dev/null +++ b/drivers/gpu/drm/tidss/tidss_dispc6.h @@ -0,0 +1,109 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> + */ + +#ifndef __TIDSS_DISPC6_H__ +#define __TIDSS_DISPC6_H__ + +/* COMMON */ + +#define DISPC_REVISION 0x000 +#define DISPC_SYSCONFIG 0x004 +#define DISPC_SYSSTATUS 0x008 + +#define DISPC_IRQ_EOI 0x020 +#define DISPC_IRQSTATUS_RAW 0x024 +#define DISPC_IRQSTATUS 0x028 +#define DISPC_IRQENABLE_SET 0x02c +#define DISPC_IRQENABLE_CLR 0x030 +#define DISPC_IRQWAKEEN 0x034 + +#define DISPC_GLOBAL_MFLAG_ATTRIBUTE 0x040 +#define DISPC_GLOBAL_BUFFER 0x044 +#define DISPC_BA0_FLIPIMMEDIATE_EN 0x048 + +#define DISPC_DBG_CONTROL 0x04c +#define DISPC_DBG_STATUS 0x050 + +#define DISPC_CLKGATING_DISABLE 0x054 + +/* VID */ + +#define DISPC_VID_ACCUH_0 0x0 +#define DISPC_VID_ACCUH_1 0x4 +#define DISPC_VID_ACCUH2_0 0x8 +#define DISPC_VID_ACCUH2_1 0xc + +#define DISPC_VID_ACCUV_0 0x10 +#define DISPC_VID_ACCUV_1 0x14 +#define DISPC_VID_ACCUV2_0 0x18 +#define DISPC_VID_ACCUV2_1 0x1c + +#define DISPC_VID_ATTRIBUTES 0x20 +#define DISPC_VID_ATTRIBUTES2 0x24 + +#define DISPC_VID_BA_0 0x28 +#define DISPC_VID_BA_1 0x2c +#define DISPC_VID_BA_UV_0 0x30 +#define DISPC_VID_BA_UV_1 0x34 +#define DISPC_VID_BUF_SIZE_STATUS 0x38 +#define DISPC_VID_BUF_THRESHOLD 0x3c + +#define DISPC_VID_CONV_COEF(n) (0x40 + (n) * 4) + +#define DISPC_VID_FIRH 0x5c +#define DISPC_VID_FIRH2 0x60 +#define DISPC_VID_FIRV 0x64 +#define DISPC_VID_FIRV2 0x68 + +#define DISPC_VID_FIR_COEFS_H0 0x6c +#define DISPC_VID_FIR_COEF_H0(phase) (0x6c + (phase) * 4) +#define DISPC_VID_FIR_COEFS_H0_C 0x90 +#define DISPC_VID_FIR_COEF_H0_C(phase) (0x90 + (phase) * 4) + +#define DISPC_VID_FIR_COEFS_H12 0xb4 +#define DISPC_VID_FIR_COEF_H12(phase) (0xb4 + (phase) * 4) +#define DISPC_VID_FIR_COEFS_H12_C 0xf4 +#define DISPC_VID_FIR_COEF_H12_C(phase) (0xf4 + (phase) * 4) + +#define DISPC_VID_FIR_COEFS_V0 0x134 +#define DISPC_VID_FIR_COEF_V0(phase) (0x134 + (phase) * 4) +#define DISPC_VID_FIR_COEFS_V0_C 0x158 +#define DISPC_VID_FIR_COEF_V0_C(phase) (0x158 + (phase) * 4) + +#define DISPC_VID_FIR_COEFS_V12 0x17c +#define DISPC_VID_FIR_COEF_V12(phase) (0x17c + (phase) * 4) +#define DISPC_VID_FIR_COEFS_V12_C 0x1bc +#define DISPC_VID_FIR_COEF_V12_C(phase) (0x1bc + (phase) * 4) + +#define DISPC_VID_IRQENABLE 0x200 +#define DISPC_VID_IRQSTATUS 0x204 + +#define DISPC_VID_MFLAG_THRESHOLD 0x208 +#define DISPC_VID_PICTURE_SIZE 0x20c +#define DISPC_VID_PIXEL_INC 0x210 +#define DISPC_VID_POSITION 0x214 +#define DISPC_VID_PRELOAD 0x218 +#define DISPC_VID_ROW_INC 0x21c +#define DISPC_VID_SIZE 0x220 + +/* OVR */ + +#define DISPC_OVR_DEFAULT_COLOR 0x08 +#define DISPC_OVR_DEFAULT_COLOR2 0x0c + +/* VP */ + +#define DISPC_VP_CONFIG 0x00 +#define DISPC_VP_CONTROL 0x04 +#define DISPC_VP_GAMMA_TABLE 0x20 +#define DISPC_VP_IRQENABLE 0x3c +#define DISPC_VP_IRQSTATUS 0x40 +#define DISPC_VP_POL_FREQ 0x4c +#define DISPC_VP_SIZE_SCREEN 0x50 +#define DISPC_VP_TIMING_H 0x54 +#define DISPC_VP_TIMING_V 0x58 + +#endif diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c new file mode 100644 index 000000000000..8002766c4640 --- /dev/null +++ b/drivers/gpu/drm/tidss/tidss_drv.c @@ -0,0 +1,333 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> + */ + +#include <linux/console.h> +#include <linux/of_device.h> +#include <linux/pm_runtime.h> + +#include <drm/drmP.h> +#include <drm/drm_atomic.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_cma_helper.h> +#include <drm/drm_fb_helper.h> +#include <drm/drm_gem_cma_helper.h> + +#include "tidss_dispc.h" +#include "tidss_drv.h" +#include "tidss_irq.h" +#include "tidss_kms.h" + +/* ----------------------------------------------------------------------------- + * Device Information + */ + +static const struct tidss_features tidss_k2g_feats = { + .dispc_init = dispc6_init, +}; + +static const struct of_device_id tidss_of_table[] = { + { .compatible = "ti,k2g-dss", .data = &tidss_k2g_feats }, + { } +}; + +DEFINE_DRM_GEM_CMA_FOPS(tidss_fops); + +static struct drm_driver tidss_driver = { + .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME + | DRIVER_ATOMIC | DRIVER_HAVE_IRQ, + .gem_free_object_unlocked = drm_gem_cma_free_object, + .gem_vm_ops = &drm_gem_cma_vm_ops, + .prime_handle_to_fd = drm_gem_prime_handle_to_fd, + .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .gem_prime_import = drm_gem_prime_import, + .gem_prime_export = drm_gem_prime_export, + .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, + .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, + .gem_prime_vmap = drm_gem_cma_prime_vmap, + .gem_prime_vunmap = drm_gem_cma_prime_vunmap, + .gem_prime_mmap = drm_gem_cma_prime_mmap, + .dumb_create = drm_gem_cma_dumb_create, + .fops = &tidss_fops, + .name = "tidss", + .desc = "TI Keystone DSS", + .date = "20180215", + .major = 1, + .minor = 0, + + .irq_preinstall = tidss_irq_preinstall, + .irq_postinstall = tidss_irq_postinstall, + .irq_handler = tidss_irq_handler, + .irq_uninstall = tidss_irq_uninstall, +}; + +#ifdef CONFIG_PM +/* ----------------------------------------------------------------------------- + * Power management + */ + +static int tidss_pm_runtime_suspend(struct device *dev) +{ + struct tidss_device *tidss = dev_get_drvdata(dev); + + dev_dbg(dev, "%s\n", __func__); + + return tidss->dispc_ops->runtime_suspend(tidss->dispc); +} + +static int tidss_pm_runtime_resume(struct device *dev) +{ + struct tidss_device *tidss = dev_get_drvdata(dev); + int r; + + dev_dbg(dev, "%s\n", __func__); + + r = tidss->dispc_ops->runtime_resume(tidss->dispc); + if (r) + return r; + + tidss_irq_resume(tidss->ddev); + + return 0; +} + +#ifdef CONFIG_PM_SLEEP +static int tidss_suspend(struct device *dev) +{ + struct tidss_device *tidss = dev_get_drvdata(dev); + struct drm_atomic_state *state; + + drm_kms_helper_poll_disable(tidss->ddev); + + console_lock(); + drm_fbdev_cma_set_suspend(tidss->fbdev, 1); + console_unlock(); + + state = drm_atomic_helper_suspend(tidss->ddev); + + if (IS_ERR(state)) { + console_lock(); + drm_fbdev_cma_set_suspend(tidss->fbdev, 0); + console_unlock(); + + drm_kms_helper_poll_enable(tidss->ddev); + + return PTR_ERR(state); + } + + tidss->saved_state = state; + + return 0; +} + +static int tidss_resume(struct device *dev) +{ + struct tidss_device *tidss = dev_get_drvdata(dev); + int ret = 0; + + if (tidss->saved_state) + ret = drm_atomic_helper_resume(tidss->ddev, tidss->saved_state); + + console_lock(); + drm_fbdev_cma_set_suspend(tidss->fbdev, 0); + console_unlock(); + + drm_kms_helper_poll_enable(tidss->ddev); + + return ret; +} +#endif /* CONFIG_PM_SLEEP */ + +static const struct dev_pm_ops tidss_pm_ops = { + .runtime_suspend = tidss_pm_runtime_suspend, + .runtime_resume = tidss_pm_runtime_resume, + SET_SYSTEM_SLEEP_PM_OPS(tidss_suspend, tidss_resume) +}; + +#endif /* CONFIG_PM */ + +/* ----------------------------------------------------------------------------- + * Platform driver + */ + +static int tidss_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct tidss_device *tidss; + struct drm_device *ddev; + int ret; + int irq; + + dev_dbg(dev, "%s\n", __func__); + + tidss = devm_kzalloc(dev, sizeof(*tidss), GFP_KERNEL); + if (tidss == NULL) + return -ENOMEM; + + tidss->pdev = pdev; + tidss->dev = dev; + tidss->feat = of_device_get_match_data(dev); + + platform_set_drvdata(pdev, tidss); + + ddev = drm_dev_alloc(&tidss_driver, dev); + if (IS_ERR(ddev)) + return PTR_ERR(ddev); + + tidss->ddev = ddev; + ddev->dev_private = tidss; + + pm_runtime_enable(dev); + + ret = tidss->feat->dispc_init(tidss); + if (ret) { + dev_err(dev, "failed to initialize dispc: %d\n", ret); + goto err_disable_pm; + } + +#ifndef CONFIG_PM_SLEEP + /* no PM, so force enable DISPC */ + tidss->dispc_ops->runtime_resume(tidss->dispc); +#endif + + ret = tidss_modeset_init(tidss); + if (ret < 0) { + if (ret != -EPROBE_DEFER) + dev_err(dev, "failed to init DRM/KMS (%d)\n", ret); + goto err_runtime_suspend; + } + + irq = platform_get_irq(tidss->pdev, 0); + if (irq < 0) { + ret = irq; + dev_err(dev, "platform_get_irq failed: %d\n", ret); + goto err_modeset_cleanup; + } + + ret = drm_irq_install(ddev, irq); + if (ret) { + dev_err(dev, "drm_irq_install failed: %d\n", ret); + goto err_modeset_cleanup; + } + +#ifdef CONFIG_DRM_FBDEV_EMULATION + if (ddev->mode_config.num_connector) { + struct drm_fbdev_cma *fbdev; + + fbdev = drm_fbdev_cma_init(ddev, 32, + ddev->mode_config.num_connector); + if (IS_ERR(fbdev)) { + dev_err(tidss->dev, "fbdev init failed\n"); + ret = PTR_ERR(fbdev); + goto err_irq_uninstall; + } + + tidss->fbdev = fbdev; + } +#endif + + drm_kms_helper_poll_init(ddev); + + ret = drm_dev_register(ddev, 0); + if (ret) { + dev_err(dev, "failed to register DRM device\n"); + goto err_poll_fini; + } + + dev_dbg(dev, "%s done\n", __func__); + + return 0; + +err_poll_fini: + drm_kms_helper_poll_fini(ddev); + + if (tidss->fbdev) + drm_fbdev_cma_fini(tidss->fbdev); + + drm_atomic_helper_shutdown(ddev); + +#ifdef CONFIG_DRM_FBDEV_EMULATION +err_irq_uninstall: +#endif + drm_irq_uninstall(ddev); + +err_modeset_cleanup: + drm_mode_config_cleanup(ddev); + +err_runtime_suspend: +#ifndef CONFIG_PM_SLEEP + /* no PM, so force disable DISPC */ + tidss->dispc_ops->runtime_suspend(tidss->dispc); +#endif + + tidss->dispc_ops->remove(tidss->dispc); + +err_disable_pm: + pm_runtime_disable(dev); + + drm_dev_put(ddev); + + return ret; +} + +static int tidss_remove(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct tidss_device *tidss = platform_get_drvdata(pdev); + struct drm_device *ddev = tidss->ddev; + + dev_dbg(dev, "%s\n", __func__); + + drm_dev_unregister(ddev); + + drm_kms_helper_poll_fini(ddev); + + if (tidss->fbdev) + drm_fbdev_cma_fini(tidss->fbdev); + + drm_atomic_helper_shutdown(ddev); + + drm_irq_uninstall(ddev); + + drm_mode_config_cleanup(ddev); + +#ifndef CONFIG_PM_SLEEP + /* no PM, so force disable DISPC */ + tidss->dispc_ops->runtime_suspend(tidss->dispc); +#endif + + tidss->dispc_ops->remove(tidss->dispc); + + pm_runtime_disable(dev); + + drm_dev_put(ddev); + + dev_dbg(dev, "%s done\n", __func__); + + return 0; +} + +MODULE_DEVICE_TABLE(of, tidss_of_table); + +static struct platform_driver tidss_platform_driver = { + .probe = tidss_probe, + .remove = tidss_remove, + .driver = { + .name = "tidss", +#ifdef CONFIG_PM + .pm = &tidss_pm_ops, +#endif + .of_match_table = tidss_of_table, + .suppress_bind_attrs = true, + }, +}; + +module_platform_driver(tidss_platform_driver); + +MODULE_AUTHOR("Tomi Valkeinen <tomi.valkeinen@ti.com>"); +MODULE_DESCRIPTION("TI Keystone DSS Driver"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/gpu/drm/tidss/tidss_drv.h b/drivers/gpu/drm/tidss/tidss_drv.h new file mode 100644 index 000000000000..fd1e767e9308 --- /dev/null +++ b/drivers/gpu/drm/tidss/tidss_drv.h @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> + */ + +#ifndef __TIDSS_DRV_H__ +#define __TIDSS_DRV_H__ + +#include <linux/spinlock.h> + +struct tidss_device { + struct device *dev; /* Underlying DSS device */ + struct platform_device *pdev; /* Underlying DSS platform device */ + struct drm_device *ddev; /* DRM device for DSS */ + + struct drm_fbdev_cma *fbdev; + + struct dispc_device *dispc; + const struct dispc_ops *dispc_ops; + + const struct tidss_features *feat; + + u32 num_crtcs; + struct drm_crtc *crtcs[8]; + + u32 num_planes; + struct drm_plane *planes[8]; + + spinlock_t wait_lock; /* protects the irq masks */ + u64 irq_mask; /* enabled irqs in addition to wait_list */ + u64 irq_uf_mask; /* underflow irq bits for all planes */ + + struct drm_atomic_state *saved_state; +}; + +struct tidss_features { + int (*dispc_init)(struct tidss_device *tidss); +}; + +#endif diff --git a/drivers/gpu/drm/tidss/tidss_encoder.c b/drivers/gpu/drm/tidss/tidss_encoder.c new file mode 100644 index 000000000000..fb7bc3fc0fe8 --- /dev/null +++ b/drivers/gpu/drm/tidss/tidss_encoder.c @@ -0,0 +1,101 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> + */ + +#include <linux/export.h> + +#include <drm/drmP.h> +#include <drm/drm_crtc.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_panel.h> +#include <drm/drm_of.h> + +#include "tidss_drv.h" +#include "tidss_encoder.h" +#include "tidss_crtc.h" + +/* ----------------------------------------------------------------------------- + * Encoder + */ + +static void tidss_encoder_disable(struct drm_encoder *encoder) +{ + struct drm_device *ddev = encoder->dev; + + dev_dbg(ddev->dev, "%s\n", __func__); + +} + +static void tidss_encoder_enable(struct drm_encoder *encoder) +{ + struct drm_device *ddev = encoder->dev; + + dev_dbg(ddev->dev, "%s\n", __func__); + +} + +static int tidss_encoder_atomic_check(struct drm_encoder *encoder, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state) +{ + struct drm_device *ddev = encoder->dev; + struct tidss_crtc_state *tcrtc_state = to_tidss_crtc_state(crtc_state); + struct drm_display_info *di = &conn_state->connector->display_info; + + dev_dbg(ddev->dev, "%s\n", __func__); + + // XXX any cleaner way to set bus format and flags? + tcrtc_state->bus_format = di->bus_formats[0]; + tcrtc_state->bus_flags = di->bus_flags; + + return 0; +} + +static void tidss_encoder_mode_set(struct drm_encoder *encoder, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state) +{ + struct drm_device *ddev = encoder->dev; + + dev_dbg(ddev->dev, "%s\n", __func__); + +} + +static const struct drm_encoder_helper_funcs encoder_helper_funcs = { + .atomic_mode_set = tidss_encoder_mode_set, + .disable = tidss_encoder_disable, + .enable = tidss_encoder_enable, + .atomic_check = tidss_encoder_atomic_check, +}; + +static const struct drm_encoder_funcs encoder_funcs = { + .destroy = drm_encoder_cleanup, +}; + +struct tidss_encoder *tidss_encoder_create(struct tidss_device *tidss, + u32 encoder_type, u32 possible_crtcs) +{ + struct tidss_encoder *tenc; + struct drm_encoder *enc; + int ret; + + tenc = devm_kzalloc(tidss->dev, sizeof(*tenc), GFP_KERNEL); + if (!tenc) + return ERR_PTR(-ENOMEM); + + enc = &tenc->encoder; + enc->possible_crtcs = possible_crtcs; + + ret = drm_encoder_init(tidss->ddev, enc, &encoder_funcs, + encoder_type, NULL); + if (ret < 0) + return ERR_PTR(ret); + + drm_encoder_helper_add(enc, &encoder_helper_funcs); + + dev_dbg(tidss->dev, "Encoder create done\n"); + + return tenc; +} diff --git a/drivers/gpu/drm/tidss/tidss_encoder.h b/drivers/gpu/drm/tidss/tidss_encoder.h new file mode 100644 index 000000000000..f1bd05a3f611 --- /dev/null +++ b/drivers/gpu/drm/tidss/tidss_encoder.h @@ -0,0 +1,22 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> + */ + +#ifndef __TIDSS_ENCODER_H__ +#define __TIDSS_ENCODER_H__ + +#include <drm/drm_encoder.h> + +struct tidss_device; +struct tidss_crtc; + +struct tidss_encoder { + struct drm_encoder encoder; +}; + +struct tidss_encoder *tidss_encoder_create(struct tidss_device *tidss, + u32 encoder_type, u32 possible_crtcs); + +#endif diff --git a/drivers/gpu/drm/tidss/tidss_irq.c b/drivers/gpu/drm/tidss/tidss_irq.c new file mode 100644 index 000000000000..07c4e1f0924e --- /dev/null +++ b/drivers/gpu/drm/tidss/tidss_irq.c @@ -0,0 +1,193 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> + */ + +#include <drm/drmP.h> + +#include "tidss_irq.h" +#include "tidss_drv.h" +#include "tidss_dispc.h" +#include "tidss_crtc.h" +#include "tidss_plane.h" + +/* call with wait_lock and dispc runtime held */ +static void tidss_irq_update(struct drm_device *ddev) +{ + struct tidss_device *tidss = ddev->dev_private; + + assert_spin_locked(&tidss->wait_lock); + + tidss->dispc_ops->write_irqenable(tidss->dispc, tidss->irq_mask); +} + +void tidss_irq_enable_vblank(struct drm_crtc *crtc) +{ + struct drm_device *ddev = crtc->dev; + struct tidss_device *tidss = ddev->dev_private; + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); + u32 hw_videoport = tcrtc->hw_videoport; + unsigned long flags; + + spin_lock_irqsave(&tidss->wait_lock, flags); + tidss->irq_mask |= DSS_IRQ_VP_VSYNC_EVEN(hw_videoport) | + DSS_IRQ_VP_VSYNC_ODD(hw_videoport); + tidss_irq_update(ddev); + spin_unlock_irqrestore(&tidss->wait_lock, flags); +} + +void tidss_irq_disable_vblank(struct drm_crtc *crtc) +{ + struct drm_device *ddev = crtc->dev; + struct tidss_device *tidss = ddev->dev_private; + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); + u32 hw_videoport = tcrtc->hw_videoport; + unsigned long flags; + + spin_lock_irqsave(&tidss->wait_lock, flags); + tidss->irq_mask &= ~(DSS_IRQ_VP_VSYNC_EVEN(hw_videoport) | + DSS_IRQ_VP_VSYNC_ODD(hw_videoport)); + tidss_irq_update(ddev); + spin_unlock_irqrestore(&tidss->wait_lock, flags); +} + +static void tidss_irq_fifo_underflow(struct tidss_device *tidss, + u64 irqstatus) +{ + static DEFINE_RATELIMIT_STATE(_rs, DEFAULT_RATELIMIT_INTERVAL, + DEFAULT_RATELIMIT_BURST); + unsigned int i; + u64 masked; + + spin_lock(&tidss->wait_lock); + masked = irqstatus & tidss->irq_uf_mask & tidss->irq_mask; + spin_unlock(&tidss->wait_lock); + + if (!masked) + return; + + if (!__ratelimit(&_rs)) + return; + + DRM_ERROR("FIFO underflow on "); + + for (i = 0; i < DSS_MAX_PLANES; ++i) { + if (masked & DSS_IRQ_PLANE_FIFO_UNDERFLOW(i)) + pr_cont("%u:%s ", i, + tidss->dispc_ops->plane_name(tidss->dispc, i)); + } + + pr_cont("(%016llx)\n", irqstatus); +} + +static void tidss_irq_ocp_error_handler(struct drm_device *ddev, + u64 irqstatus) +{ + if (irqstatus & DSS_IRQ_DEVICE_OCP_ERR) + dev_err_ratelimited(ddev->dev, "OCP error\n"); +} + +irqreturn_t tidss_irq_handler(int irq, void *arg) +{ + struct drm_device *ddev = (struct drm_device *) arg; + struct tidss_device *tidss = ddev->dev_private; + unsigned int id; + u64 irqstatus; + + if (WARN_ON(!ddev->irq_enabled)) + return IRQ_NONE; + + irqstatus = tidss->dispc_ops->read_and_clear_irqstatus(tidss->dispc); + + for (id = 0; id < tidss->num_crtcs; id++) { + struct drm_crtc *crtc = tidss->crtcs[id]; + struct tidss_crtc *tcrtc = to_tidss_crtc(crtc); + u32 hw_videoport = tcrtc->hw_videoport; + + if (irqstatus & (DSS_IRQ_VP_VSYNC_EVEN(hw_videoport) | + DSS_IRQ_VP_VSYNC_ODD(hw_videoport))) + tidss_crtc_vblank_irq(crtc); + + if (irqstatus & (DSS_IRQ_VP_FRAME_DONE(hw_videoport))) + tidss_crtc_framedone_irq(crtc); + + if (irqstatus & DSS_IRQ_VP_SYNC_LOST(hw_videoport)) + tidss_crtc_error_irq(crtc, irqstatus); + } + + tidss_irq_ocp_error_handler(ddev, irqstatus); + tidss_irq_fifo_underflow(tidss, irqstatus); + + return IRQ_HANDLED; +} + +void tidss_irq_preinstall(struct drm_device *ddev) +{ + struct tidss_device *tidss = ddev->dev_private; + + spin_lock_init(&tidss->wait_lock); + + tidss->dispc_ops->runtime_get(tidss->dispc); + + tidss->dispc_ops->write_irqenable(tidss->dispc, 0); + tidss->dispc_ops->read_and_clear_irqstatus(tidss->dispc); + + tidss->dispc_ops->runtime_put(tidss->dispc); +} + +int tidss_irq_postinstall(struct drm_device *ddev) +{ + struct tidss_device *tidss = ddev->dev_private; + unsigned int i; + unsigned long flags; + + tidss->dispc_ops->runtime_get(tidss->dispc); + + spin_lock_irqsave(&tidss->wait_lock, flags); + + tidss->irq_mask = DSS_IRQ_DEVICE_OCP_ERR; + + tidss->irq_uf_mask = 0; + for (i = 0; i < tidss->num_planes; ++i) { + struct tidss_plane *tplane = to_tidss_plane(tidss->planes[i]); + + tidss->irq_uf_mask |= DSS_IRQ_PLANE_FIFO_UNDERFLOW(tplane->hw_plane_id); + } + tidss->irq_mask |= tidss->irq_uf_mask; + + for (i = 0; i < tidss->num_crtcs; ++i) { + struct tidss_crtc *tcrtc = to_tidss_crtc(tidss->crtcs[i]); + + tidss->irq_mask |= DSS_IRQ_VP_SYNC_LOST(tcrtc->hw_videoport); + + tidss->irq_mask |= DSS_IRQ_VP_FRAME_DONE(tcrtc->hw_videoport); + } + + tidss_irq_update(ddev); + + spin_unlock_irqrestore(&tidss->wait_lock, flags); + + tidss->dispc_ops->runtime_put(tidss->dispc); + + return 0; +} + +void tidss_irq_uninstall(struct drm_device *ddev) +{ + struct tidss_device *tidss = ddev->dev_private; + + tidss->dispc_ops->runtime_get(tidss->dispc); + tidss->dispc_ops->write_irqenable(tidss->dispc, 0); + tidss->dispc_ops->runtime_put(tidss->dispc); +} + +void tidss_irq_resume(struct drm_device *ddev) +{ + struct tidss_device *tidss = ddev->dev_private; + unsigned long flags; + + spin_lock_irqsave(&tidss->wait_lock, flags); + tidss_irq_update(ddev); + spin_unlock_irqrestore(&tidss->wait_lock, flags); +} diff --git a/drivers/gpu/drm/tidss/tidss_irq.h b/drivers/gpu/drm/tidss/tidss_irq.h new file mode 100644 index 000000000000..e1507298c9d8 --- /dev/null +++ b/drivers/gpu/drm/tidss/tidss_irq.h @@ -0,0 +1,25 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> + */ + +#ifndef __TIDSS_IRQ_H__ +#define __TIDSS_IRQ_H__ + +#include <linux/types.h> + +struct drm_crtc; +struct drm_device; + +void tidss_irq_enable_vblank(struct drm_crtc *crtc); +void tidss_irq_disable_vblank(struct drm_crtc *crtc); + +void tidss_irq_preinstall(struct drm_device *ddev); +int tidss_irq_postinstall(struct drm_device *ddev); +void tidss_irq_uninstall(struct drm_device *ddev); +irqreturn_t tidss_irq_handler(int irq, void *arg); + +void tidss_irq_resume(struct drm_device *ddev); + +#endif diff --git a/drivers/gpu/drm/tidss/tidss_kms.c b/drivers/gpu/drm/tidss/tidss_kms.c new file mode 100644 index 000000000000..cd003f1569cd --- /dev/null +++ b/drivers/gpu/drm/tidss/tidss_kms.c @@ -0,0 +1,85 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> + */ + +#include <drm/drm_atomic.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_helper.h> +#include <drm/drm_fb_cma_helper.h> +#include <drm/drm_gem_framebuffer_helper.h> + +#include "tidss_crtc.h" +#include "tidss_drv.h" +#include "tidss_encoder.h" +#include "tidss_kms.h" +#include "tidss_plane.h" + +static void tidss_atomic_commit_tail(struct drm_atomic_state *old_state) +{ + struct drm_device *ddev = old_state->dev; + struct tidss_device *tidss = ddev->dev_private; + + dev_dbg(ddev->dev, "%s\n", __func__); + + tidss->dispc_ops->runtime_get(tidss->dispc); + + drm_atomic_helper_commit_modeset_disables(ddev, old_state); + drm_atomic_helper_commit_planes(ddev, old_state, 0); + drm_atomic_helper_commit_modeset_enables(ddev, old_state); + + drm_atomic_helper_commit_hw_done(old_state); + drm_atomic_helper_wait_for_flip_done(ddev, old_state); + + drm_atomic_helper_cleanup_planes(ddev, old_state); + + tidss->dispc_ops->runtime_put(tidss->dispc); +} + +static const struct drm_mode_config_helper_funcs mode_config_helper_funcs = { + .atomic_commit_tail = tidss_atomic_commit_tail, +}; + +static const struct drm_mode_config_funcs mode_config_funcs = { + .fb_create = drm_gem_fb_create, + .atomic_check = drm_atomic_helper_check, + .atomic_commit = drm_atomic_helper_commit, +}; + +int tidss_modeset_init(struct tidss_device *tidss) +{ + struct drm_device *ddev = tidss->ddev; + int ret; + int i; + + dev_dbg(tidss->dev, "%s\n", __func__); + + drm_mode_config_init(ddev); + + ddev->mode_config.min_width = 8; + ddev->mode_config.min_height = 8; + ddev->mode_config.max_width = 8096; + ddev->mode_config.max_height = 8096; + ddev->mode_config.funcs = &mode_config_funcs; + ddev->mode_config.helper_private = &mode_config_helper_funcs; + + ret = tidss->dispc_ops->modeset_init(tidss->dispc); + if (ret) + return ret; + + ret = drm_vblank_init(ddev, tidss->num_crtcs); + if (ret) + return ret; + + /* Start with vertical blanking interrupt reporting disabled. */ + for (i = 0; i < tidss->num_crtcs; ++i) + drm_crtc_vblank_reset(tidss->crtcs[i]); + + drm_mode_config_reset(ddev); + + dev_dbg(tidss->dev, "%s done\n", __func__); + + return 0; +} diff --git a/drivers/gpu/drm/tidss/tidss_kms.h b/drivers/gpu/drm/tidss/tidss_kms.h new file mode 100644 index 000000000000..927b57b05827 --- /dev/null +++ b/drivers/gpu/drm/tidss/tidss_kms.h @@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> + */ + +#ifndef __TIDSS_KMS_H__ +#define __TIDSS_KMS_H__ + +struct tidss_device; + +int tidss_modeset_init(struct tidss_device *rcdu); + +#endif diff --git a/drivers/gpu/drm/tidss/tidss_plane.c b/drivers/gpu/drm/tidss/tidss_plane.c new file mode 100644 index 000000000000..539c3ae706de --- /dev/null +++ b/drivers/gpu/drm/tidss/tidss_plane.c @@ -0,0 +1,186 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> + */ + +#include <drm/drmP.h> +#include <drm/drm_atomic.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_cma_helper.h> +#include <drm/drm_gem_cma_helper.h> +#include <drm/drm_plane_helper.h> + +#include "tidss_crtc.h" +#include "tidss_drv.h" +#include "tidss_plane.h" + +static int tidss_plane_atomic_check(struct drm_plane *plane, + struct drm_plane_state *state) +{ + struct drm_device *ddev = plane->dev; + struct drm_crtc_state *crtc_state; + int ret; + + dev_dbg(ddev->dev, "%s\n", __func__); + + if (!state->crtc) { + /* + * The visible field is not reset by the DRM core but only + * updated by drm_plane_helper_check_state(), set it manually. + */ + state->visible = false; + return 0; + } + + crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc); + if (IS_ERR(crtc_state)) + return PTR_ERR(crtc_state); + + /* XXX TODO: check scaling via dispc_ops */ + + ret = drm_atomic_helper_check_plane_state(state, crtc_state, + 0, + INT_MAX, + true, true); + if (ret < 0) + return ret; + + if (!state->visible) + return 0; + + return 0; +} + +static void tidss_plane_atomic_update(struct drm_plane *plane, + struct drm_plane_state *old_state) +{ + struct drm_device *ddev = plane->dev; + struct tidss_device *tidss = ddev->dev_private; + struct tidss_plane *tplane = to_tidss_plane(plane); + struct tidss_plane_info info; + int ret; + u32 hw_videoport; + struct drm_plane_state *state = plane->state; + struct drm_framebuffer *fb = state->fb; + uint32_t x, y; + struct drm_gem_cma_object *gem; + + dev_dbg(ddev->dev, "%s\n", __func__); + + if (!state->visible) { + tidss->dispc_ops->plane_enable(tidss->dispc, tplane->hw_plane_id, + false); + return; + } + + hw_videoport = to_tidss_crtc(state->crtc)->hw_videoport; + + memset(&info, 0, sizeof(info)); + + info.fourcc = fb->format->format; + info.pos_x = state->crtc_x; + info.pos_y = state->crtc_y; + info.out_width = state->crtc_w; + info.out_height = state->crtc_h; + info.width = state->src_w >> 16; + info.height = state->src_h >> 16; + info.zorder = state->zpos; + + x = state->src_x >> 16; + y = state->src_y >> 16; + + gem = drm_fb_cma_get_gem_obj(fb, 0); + + info.paddr = gem->paddr + fb->offsets[0] + x * fb->format->cpp[0] + + y * fb->pitches[0]; + + info.fb_width = fb->pitches[0] / fb->format->cpp[0]; + + if (fb->format->num_planes == 2) { + gem = drm_fb_cma_get_gem_obj(fb, 1); + + info.p_uv_addr = gem->paddr + + fb->offsets[0] + + (x * fb->format->cpp[0] / fb->format->hsub) + + (y * fb->pitches[0] / fb->format->vsub); + } + + ret = tidss->dispc_ops->plane_setup(tidss->dispc, tplane->hw_plane_id, + &info, hw_videoport); + + if (ret) { + dev_err(plane->dev->dev, "Failed to setup plane %d\n", + tplane->hw_plane_id); + tidss->dispc_ops->plane_enable(tidss->dispc, tplane->hw_plane_id, + false); + return; + } + + tidss->dispc_ops->plane_enable(tidss->dispc, tplane->hw_plane_id, true); +} + +static void tidss_plane_atomic_disable(struct drm_plane *plane, + struct drm_plane_state *old_state) +{ + struct drm_device *ddev = plane->dev; + struct tidss_device *tidss = ddev->dev_private; + struct tidss_plane *tplane = to_tidss_plane(plane); + + dev_dbg(ddev->dev, "%s\n", __func__); + + tidss->dispc_ops->plane_enable(tidss->dispc, tplane->hw_plane_id, false); +} + +static const struct drm_plane_helper_funcs tidss_plane_helper_funcs = { + .atomic_check = tidss_plane_atomic_check, + .atomic_update = tidss_plane_atomic_update, + .atomic_disable = tidss_plane_atomic_disable, +}; + +static const struct drm_plane_funcs tidss_plane_funcs = { + .update_plane = drm_atomic_helper_update_plane, + .disable_plane = drm_atomic_helper_disable_plane, + .reset = drm_atomic_helper_plane_reset, + .destroy = drm_plane_cleanup, + .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, +}; + +struct tidss_plane *tidss_plane_create(struct tidss_device *tidss, + u32 hw_plane_id, u32 plane_type, + u32 crtc_mask, const u32 *formats, + u32 num_formats) +{ + enum drm_plane_type type; + uint32_t possible_crtcs; + uint num_planes = tidss->dispc_ops->get_num_planes(tidss->dispc); + int ret; + struct tidss_plane *tplane; + + tplane = devm_kzalloc(tidss->dev, sizeof(*tplane), GFP_KERNEL); + if (!tplane) + return ERR_PTR(-ENOMEM); + + tplane->hw_plane_id = hw_plane_id; + + possible_crtcs = crtc_mask; + type = plane_type; + + ret = drm_universal_plane_init(tidss->ddev, &tplane->plane, + possible_crtcs, + &tidss_plane_funcs, + formats, num_formats, + NULL, type, NULL); + if (ret < 0) + return ERR_PTR(ret); + + drm_plane_helper_add(&tplane->plane, &tidss_plane_helper_funcs); + if (num_planes > 1) + drm_plane_create_zpos_property(&tplane->plane, hw_plane_id, 0, + num_planes - 1); + + return tplane; +} diff --git a/drivers/gpu/drm/tidss/tidss_plane.h b/drivers/gpu/drm/tidss/tidss_plane.h new file mode 100644 index 000000000000..d865e14cb5f9 --- /dev/null +++ b/drivers/gpu/drm/tidss/tidss_plane.h @@ -0,0 +1,25 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> + */ + +#ifndef __TIDSS_PLANE_H__ +#define __TIDSS_PLANE_H__ + +#include "tidss_dispc.h" + +#define to_tidss_plane(p) container_of((p), struct tidss_plane, plane) + +struct tidss_plane { + struct drm_plane plane; + + u32 hw_plane_id; + +}; + +struct tidss_plane *tidss_plane_create(struct tidss_device *tidss, + u32 hw_plane_id, u32 plane_type, + u32 crtc_mask, const u32 *formats, + u32 num_formats); +#endif
This patch adds a new DRM driver for Texas Instruments DSS6 IP used on Texas Instruments Keystone K2G SoC. The DSS6 IP is a major change to the older DSS IP versions, which are supported by the omapdrm driver, and while on higher level the DSS6 resembles the older DSS versions, the registers and the internal pipelines differ a lot. DSS6 IP on K2G is a "ultra-light" version, and has only a single plane and a single output. The driver will also support future DSS versions, which support multiple planes and outputs, so the driver already has support for those. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/tidss/Kconfig | 10 + drivers/gpu/drm/tidss/Makefile | 11 + drivers/gpu/drm/tidss/tidss_crtc.c | 390 +++++++ drivers/gpu/drm/tidss/tidss_crtc.h | 49 + drivers/gpu/drm/tidss/tidss_dispc.h | 145 +++ drivers/gpu/drm/tidss/tidss_dispc6.c | 1450 +++++++++++++++++++++++++ drivers/gpu/drm/tidss/tidss_dispc6.h | 109 ++ drivers/gpu/drm/tidss/tidss_drv.c | 333 ++++++ drivers/gpu/drm/tidss/tidss_drv.h | 41 + drivers/gpu/drm/tidss/tidss_encoder.c | 101 ++ drivers/gpu/drm/tidss/tidss_encoder.h | 22 + drivers/gpu/drm/tidss/tidss_irq.c | 193 ++++ drivers/gpu/drm/tidss/tidss_irq.h | 25 + drivers/gpu/drm/tidss/tidss_kms.c | 85 ++ drivers/gpu/drm/tidss/tidss_kms.h | 14 + drivers/gpu/drm/tidss/tidss_plane.c | 186 ++++ drivers/gpu/drm/tidss/tidss_plane.h | 25 + 19 files changed, 3192 insertions(+) create mode 100644 drivers/gpu/drm/tidss/Kconfig create mode 100644 drivers/gpu/drm/tidss/Makefile create mode 100644 drivers/gpu/drm/tidss/tidss_crtc.c create mode 100644 drivers/gpu/drm/tidss/tidss_crtc.h create mode 100644 drivers/gpu/drm/tidss/tidss_dispc.h create mode 100644 drivers/gpu/drm/tidss/tidss_dispc6.c create mode 100644 drivers/gpu/drm/tidss/tidss_dispc6.h create mode 100644 drivers/gpu/drm/tidss/tidss_drv.c create mode 100644 drivers/gpu/drm/tidss/tidss_drv.h create mode 100644 drivers/gpu/drm/tidss/tidss_encoder.c create mode 100644 drivers/gpu/drm/tidss/tidss_encoder.h create mode 100644 drivers/gpu/drm/tidss/tidss_irq.c create mode 100644 drivers/gpu/drm/tidss/tidss_irq.h create mode 100644 drivers/gpu/drm/tidss/tidss_kms.c create mode 100644 drivers/gpu/drm/tidss/tidss_kms.h create mode 100644 drivers/gpu/drm/tidss/tidss_plane.c create mode 100644 drivers/gpu/drm/tidss/tidss_plane.h