LinuxLists.cc - [PATCHv6 0/4] omapdrm: DSI command mode panel support

[permalink] [raw]

Subject: [PATCHv6 2/4] drm/omap: don't check dispc timings for DSI

While most display types only forward their VM to the DISPC, this
is not true for DSI. DSI calculates the VM for DISPC based on its
own, but it's not identical. Actually the DSI VM is not even a valid
DISPC VM making this check fail. Let's restore the old behaviour
and avoid checking the DISPC VM for DSI here.

Fixes: 7c27fa57ef31 ("drm/omap: Call dispc timings check operation directly")
Acked-by: Pavel Machek <[email protected]>
Tested-by: Tony Lindgren <[email protected]>
Tested-by: Pavel Machek <[email protected]>
Signed-off-by: Sebastian Reichel <[email protected]>
---
drivers/gpu/drm/omapdrm/omap_crtc.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 5a29bf01c0e8..86827a061b0b 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -395,10 +395,20 @@ static enum drm_mode_status omap_crtc_mode_valid(struct drm_crtc *crtc,
int r;

drm_display_mode_to_videomode(mode, &vm);
- r = priv->dispc_ops->mgr_check_timings(priv->dispc, omap_crtc->channel,
- &vm);
- if (r)
- return r;
+
+ /*
+ * DSI might not call this, since the supplied mode is not a
+ * valid DISPC mode. DSI will calculate and configure the
+ * proper DISPC mode later.
+ */
+ if (omap_crtc->pipe->output->next == NULL ||
+ omap_crtc->pipe->output->next->type != OMAP_DISPLAY_TYPE_DSI) {
+ r = priv->dispc_ops->mgr_check_timings(priv->dispc,
+ omap_crtc->channel,
+ &vm);
+ if (r)
+ return r;
+ }

/* Check for bandwidth limit */
if (priv->max_bandwidth) {
--
2.20.1

2019-05-23 20:10:05

[permalink] [raw]

Subject: [PATCHv6 3/4] drm/omap: add framedone interrupt support

This prepares framedone interrupt handling for
manual display update support.

Acked-by: Pavel Machek <[email protected]>
Tested-by: Tony Lindgren <[email protected]>
Tested-by: Pavel Machek <[email protected]>
Signed-off-by: Sebastian Reichel <[email protected]>
---
drivers/gpu/drm/omapdrm/omap_crtc.c | 50 +++++++++++++++++++++++++++++
drivers/gpu/drm/omapdrm/omap_crtc.h | 1 +
drivers/gpu/drm/omapdrm/omap_irq.c | 25 +++++++++++++++
drivers/gpu/drm/omapdrm/omap_irq.h | 1 +
4 files changed, 77 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 86827a061b0b..68697820d189 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -51,6 +51,9 @@ struct omap_crtc {
bool pending;
wait_queue_head_t pending_wait;
struct drm_pending_vblank_event *event;
+
+ void (*framedone_handler)(void *);
+ void *framedone_handler_data;
};

/* -----------------------------------------------------------------------------
@@ -230,6 +233,18 @@ static int omap_crtc_dss_register_framedone(
struct omap_drm_private *priv, enum omap_channel channel,
void (*handler)(void *), void *data)
{
+ struct drm_crtc *crtc = priv->channels[channel]->crtc;
+ struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+ struct drm_device *dev = omap_crtc->base.dev;
+
+ if (omap_crtc->framedone_handler)
+ return -EBUSY;
+
+ dev_dbg(dev->dev, "register framedone %s", omap_crtc->name);
+
+ omap_crtc->framedone_handler = handler;
+ omap_crtc->framedone_handler_data = data;
+
return 0;
}

@@ -237,6 +252,17 @@ static void omap_crtc_dss_unregister_framedone(
struct omap_drm_private *priv, enum omap_channel channel,
void (*handler)(void *), void *data)
{
+ struct drm_crtc *crtc = priv->channels[channel]->crtc;
+ struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+ struct drm_device *dev = omap_crtc->base.dev;
+
+ dev_dbg(dev->dev, "unregister framedone %s", omap_crtc->name);
+
+ WARN_ON(omap_crtc->framedone_handler != handler);
+ WARN_ON(omap_crtc->framedone_handler_data != data);
+
+ omap_crtc->framedone_handler = NULL;
+ omap_crtc->framedone_handler_data = NULL;
}

static const struct dss_mgr_ops mgr_ops = {
@@ -302,6 +328,30 @@ void omap_crtc_vblank_irq(struct drm_crtc *crtc)
DBG("%s: apply done", omap_crtc->name);
}

+void omap_crtc_framedone_irq(struct drm_crtc *crtc, uint32_t irqstatus)
+{
+ struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+
+ if (!omap_crtc->framedone_handler) {
+ dev_warn(omap_crtc->base.dev->dev, "no framedone handler?");
+ return;
+ }
+
+ omap_crtc->framedone_handler(omap_crtc->framedone_handler_data);
+
+ spin_lock(&crtc->dev->event_lock);
+ /* Send the vblank event if one has been requested. */
+ if (omap_crtc->event) {
+ drm_crtc_send_vblank_event(crtc, omap_crtc->event);
+ omap_crtc->event = NULL;
+ }
+ omap_crtc->pending = false;
+ spin_unlock(&crtc->dev->event_lock);
+
+ /* Wake up omap_atomic_complete. */
+ wake_up(&omap_crtc->pending_wait);
+}
+
static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc)
{
struct omap_drm_private *priv = crtc->dev->dev_private;
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.h b/drivers/gpu/drm/omapdrm/omap_crtc.h
index d9de437ba9dd..d33bbb7a4f90 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.h
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.h
@@ -41,5 +41,6 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
int omap_crtc_wait_pending(struct drm_crtc *crtc);
void omap_crtc_error_irq(struct drm_crtc *crtc, u32 irqstatus);
void omap_crtc_vblank_irq(struct drm_crtc *crtc);
+void omap_crtc_framedone_irq(struct drm_crtc *crtc, uint32_t irqstatus);

#endif /* __OMAPDRM_CRTC_H__ */
diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c b/drivers/gpu/drm/omapdrm/omap_irq.c
index 329ad26d6d50..01dda84ca2ee 100644
--- a/drivers/gpu/drm/omapdrm/omap_irq.c
+++ b/drivers/gpu/drm/omapdrm/omap_irq.c
@@ -85,6 +85,28 @@ int omap_irq_wait(struct drm_device *dev, struct omap_irq_wait *wait,
return ret == 0 ? -1 : 0;
}

+int omap_irq_enable_framedone(struct drm_crtc *crtc, bool enable)
+{
+ struct drm_device *dev = crtc->dev;
+ struct omap_drm_private *priv = dev->dev_private;
+ unsigned long flags;
+ enum omap_channel channel = omap_crtc_channel(crtc);
+ int framedone_irq =
+ priv->dispc_ops->mgr_get_framedone_irq(priv->dispc, channel);
+
+ DBG("dev=%p, crtc=%u, enable=%d", dev, channel, enable);
+
+ spin_lock_irqsave(&priv->wait_lock, flags);
+ if (enable)
+ priv->irq_mask |= framedone_irq;
+ else
+ priv->irq_mask &= ~framedone_irq;
+ omap_irq_update(dev);
+ spin_unlock_irqrestore(&priv->wait_lock, flags);
+
+ return 0;
+}
+
/**
* enable_vblank - enable vblank interrupt events
* @dev: DRM device
@@ -217,6 +239,9 @@ static irqreturn_t omap_irq_handler(int irq, void *arg)

if (irqstatus & priv->dispc_ops->mgr_get_sync_lost_irq(priv->dispc, channel))
omap_crtc_error_irq(crtc, irqstatus);
+
+ if (irqstatus & priv->dispc_ops->mgr_get_framedone_irq(priv->dispc, channel))
+ omap_crtc_framedone_irq(crtc, irqstatus);
}

omap_irq_ocp_error_handler(dev, irqstatus);
diff --git a/drivers/gpu/drm/omapdrm/omap_irq.h b/drivers/gpu/drm/omapdrm/omap_irq.h
index 9d5441468eca..02abb4ed9813 100644
--- a/drivers/gpu/drm/omapdrm/omap_irq.h
+++ b/drivers/gpu/drm/omapdrm/omap_irq.h
@@ -27,6 +27,7 @@ struct drm_device;
struct omap_irq_wait;

int omap_irq_enable_vblank(struct drm_crtc *crtc);
+int omap_irq_enable_framedone(struct drm_crtc *crtc, bool enable);
void omap_irq_disable_vblank(struct drm_crtc *crtc);
void omap_drm_irq_uninstall(struct drm_device *dev);
int omap_drm_irq_install(struct drm_device *dev);
--
2.20.1

2019-05-23 20:11:49

[permalink] [raw]

Subject: [PATCHv6 4/4] drm/omap: add support for manually updated displays

This adds the required infrastructure for manually updated displays,
such as DSI command mode panels. While those panels often support
partial updates we currently always do a full refresh.

The display will be refreshed when something calls the dirty callback,
such as libdrm's drmModeDirtyFB(). This is currently being done at least
by the kernel console and Xorg (with modesetting driver) in their
default configuration. Weston does not implement this and the fbdev
backend does not work (display will not update). Weston's DRM backend
uses double buffering and the page flip will also trigger a display
refresh.

Signed-off-by: Sebastian Reichel <[email protected]>
---
drivers/gpu/drm/omapdrm/omap_crtc.c | 114 ++++++++++++++++++++++++++--
drivers/gpu/drm/omapdrm/omap_crtc.h | 1 +
drivers/gpu/drm/omapdrm/omap_fb.c | 19 +++++
3 files changed, 129 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 68697820d189..b59065365c9e 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -32,6 +32,7 @@ struct omap_crtc_state {
/* Shadow values for legacy userspace support. */
unsigned int rotation;
unsigned int zpos;
+ bool manually_updated;
};

#define to_omap_crtc(x) container_of(x, struct omap_crtc, base)
@@ -51,6 +52,7 @@ struct omap_crtc {
bool pending;
wait_queue_head_t pending_wait;
struct drm_pending_vblank_event *event;
+ struct delayed_work update_work;

void (*framedone_handler)(void *);
void *framedone_handler_data;
@@ -105,21 +107,18 @@ int omap_crtc_wait_pending(struct drm_crtc *crtc)
/*
* Manager-ops, callbacks from output when they need to configure
* the upstream part of the video pipe.
- *
- * Most of these we can ignore until we add support for command-mode
- * panels.. for video-mode the crtc-helpers already do an adequate
- * job of sequencing the setup of the video pipe in the proper order
*/

-/* we can probably ignore these until we support command-mode panels: */
static void omap_crtc_dss_start_update(struct omap_drm_private *priv,
enum omap_channel channel)
{
+ priv->dispc_ops->mgr_enable(priv->dispc, channel, true);
}

/* Called only from the encoder enable/disable and suspend/resume handlers. */
static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
{
+ struct omap_crtc_state *omap_state = to_omap_crtc_state(crtc->state);
struct drm_device *dev = crtc->dev;
struct omap_drm_private *priv = dev->dev_private;
struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
@@ -131,6 +130,12 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
if (WARN_ON(omap_crtc->enabled == enable))
return;

+ if (omap_state->manually_updated) {
+ omap_irq_enable_framedone(crtc, enable);
+ omap_crtc->enabled = enable;
+ return;
+ }
+
if (omap_crtc->pipe->output->type == OMAP_DISPLAY_TYPE_HDMI) {
priv->dispc_ops->mgr_enable(priv->dispc, channel, enable);
omap_crtc->enabled = enable;
@@ -352,6 +357,51 @@ void omap_crtc_framedone_irq(struct drm_crtc *crtc, uint32_t irqstatus)
wake_up(&omap_crtc->pending_wait);
}

+void omap_crtc_flush(struct drm_crtc *crtc)
+{
+ struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+ struct omap_crtc_state *omap_state = to_omap_crtc_state(crtc->state);
+
+ if (!omap_state->manually_updated)
+ return;
+
+ if (!delayed_work_pending(&omap_crtc->update_work))
+ schedule_delayed_work(&omap_crtc->update_work, 0);
+}
+
+static void omap_crtc_manual_display_update(struct work_struct *data)
+{
+ struct omap_crtc *omap_crtc =
+ container_of(data, struct omap_crtc, update_work.work);
+ struct drm_display_mode *mode = &omap_crtc->pipe->crtc->mode;
+ struct omap_dss_device *dssdev = omap_crtc->pipe->output->next;
+ struct drm_device *dev = omap_crtc->base.dev;
+ const struct omap_dss_driver *dssdrv;
+ int ret;
+
+ if (!dssdev) {
+ dev_err_once(dev->dev, "missing display dssdev!");
+ return;
+ }
+
+ dssdrv = dssdev->driver;
+ if (!dssdrv || !dssdrv->update) {
+ dev_err_once(dev->dev, "missing or incorrect dssdrv!");
+ return;
+ }
+
+ if (dssdrv->sync)
+ dssdrv->sync(dssdev);
+
+ ret = dssdrv->update(dssdev, 0, 0, mode->hdisplay, mode->vdisplay);
+ if (ret < 0) {
+ spin_lock_irq(&dev->event_lock);
+ omap_crtc->pending = false;
+ spin_unlock_irq(&dev->event_lock);
+ wake_up(&omap_crtc->pending_wait);
+ }
+}
+
static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc)
{
struct omap_drm_private *priv = crtc->dev->dev_private;
@@ -401,12 +451,17 @@ static void omap_crtc_atomic_enable(struct drm_crtc *crtc,
{
struct omap_drm_private *priv = crtc->dev->dev_private;
struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+ struct omap_crtc_state *omap_state = to_omap_crtc_state(crtc->state);
int ret;

DBG("%s", omap_crtc->name);

priv->dispc_ops->runtime_get(priv->dispc);

+ /* manual updated display will not trigger vsync irq */
+ if (omap_state->manually_updated)
+ return;
+
spin_lock_irq(&crtc->dev->event_lock);
drm_crtc_vblank_on(crtc);
ret = drm_crtc_vblank_get(crtc);
@@ -421,6 +476,7 @@ static void omap_crtc_atomic_disable(struct drm_crtc *crtc,
{
struct omap_drm_private *priv = crtc->dev->dev_private;
struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+ struct drm_device *dev = crtc->dev;

DBG("%s", omap_crtc->name);

@@ -431,6 +487,11 @@ static void omap_crtc_atomic_disable(struct drm_crtc *crtc,
}
spin_unlock_irq(&crtc->dev->event_lock);

+ cancel_delayed_work(&omap_crtc->update_work);
+
+ if (!omap_crtc_wait_pending(crtc))
+ dev_warn(dev->dev, "manual display update did not finish!");
+
drm_crtc_vblank_off(crtc);

priv->dispc_ops->runtime_put(priv->dispc);
@@ -501,6 +562,22 @@ static void omap_crtc_mode_set_nofb(struct drm_crtc *crtc)
drm_display_mode_to_videomode(mode, &omap_crtc->vm);
}

+static bool omap_crtc_is_manually_updated(struct drm_crtc *crtc)
+{
+ struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+ struct omap_dss_device *display = omap_crtc->pipe->output->next;
+
+ if (!display)
+ return false;
+
+ if (display->caps & OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE) {
+ DBG("detected manually updated display!");
+ return true;
+ }
+
+ return false;
+}
+
static int omap_crtc_atomic_check(struct drm_crtc *crtc,
struct drm_crtc_state *state)
{
@@ -522,6 +599,9 @@ static int omap_crtc_atomic_check(struct drm_crtc *crtc,
/* Mirror new values for zpos and rotation in omap_crtc_state */
omap_crtc_state->zpos = pri_state->zpos;
omap_crtc_state->rotation = pri_state->rotation;
+
+ /* Check if this CRTC is for a manually updated display */
+ omap_crtc_state->manually_updated = omap_crtc_is_manually_updated(crtc);
}

return 0;
@@ -537,6 +617,7 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
{
struct omap_drm_private *priv = crtc->dev->dev_private;
struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+ struct omap_crtc_state *omap_crtc_state = to_omap_crtc_state(crtc->state);
int ret;

if (crtc->state->color_mgmt_changed) {
@@ -561,6 +642,15 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,

DBG("%s: GO", omap_crtc->name);

+ if (omap_crtc_state->manually_updated) {
+ /* send new image for page flips and modeset changes */
+ spin_lock_irq(&crtc->dev->event_lock);
+ omap_crtc_flush(crtc);
+ omap_crtc_arm_event(crtc);
+ spin_unlock_irq(&crtc->dev->event_lock);
+ return;
+ }
+
ret = drm_crtc_vblank_get(crtc);
WARN_ON(ret != 0);

@@ -646,6 +736,7 @@ omap_crtc_duplicate_state(struct drm_crtc *crtc)

state->zpos = current_state->zpos;
state->rotation = current_state->rotation;
+ state->manually_updated = current_state->manually_updated;

return &state->base;
}
@@ -722,6 +813,19 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
omap_crtc->channel = channel;
omap_crtc->name = channel_names[channel];

+ /*
+ * We want to refresh manually updated displays from dirty callback,
+ * which is called quite often (e.g. for each drawn line). This will
+ * be used to do the display update asynchronously to avoid blocking
+ * the rendering process and merges multiple dirty calls into one
+ * update if they arrive very fast. We also call this function for
+ * atomic display updates (e.g. for page flips), which means we do
+ * not need extra locking. Atomic updates should be synchronous, but
+ * need to wait for the framedone interrupt anyways.
+ */
+ INIT_DELAYED_WORK(&omap_crtc->update_work,
+ omap_crtc_manual_display_update);
+
ret = drm_crtc_init_with_planes(dev, crtc, plane, NULL,
&omap_crtc_funcs, NULL);
if (ret < 0) {
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.h b/drivers/gpu/drm/omapdrm/omap_crtc.h
index d33bbb7a4f90..2b518c74203e 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.h
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.h
@@ -42,5 +42,6 @@ int omap_crtc_wait_pending(struct drm_crtc *crtc);
void omap_crtc_error_irq(struct drm_crtc *crtc, u32 irqstatus);
void omap_crtc_vblank_irq(struct drm_crtc *crtc);
void omap_crtc_framedone_irq(struct drm_crtc *crtc, uint32_t irqstatus);
+void omap_crtc_flush(struct drm_crtc *crtc);

#endif /* __OMAPDRM_CRTC_H__ */
diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
index 4f8eb9d08f99..b6f052f2a7e1 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -66,8 +66,27 @@ struct omap_framebuffer {
struct mutex lock;
};

+static int omap_framebuffer_dirty(struct drm_framebuffer *fb,
+ struct drm_file *file_priv,
+ unsigned flags, unsigned color,
+ struct drm_clip_rect *clips,
+ unsigned num_clips)
+{
+ struct drm_crtc *crtc;
+
+ drm_modeset_lock_all(fb->dev);
+
+ drm_for_each_crtc(crtc, fb->dev)
+ omap_crtc_flush(crtc);
+
+ drm_modeset_unlock_all(fb->dev);
+
+ return 0;
+}
+
static const struct drm_framebuffer_funcs omap_framebuffer_funcs = {
.create_handle = drm_gem_fb_create_handle,
+ .dirty = omap_framebuffer_dirty,
.destroy = drm_gem_fb_destroy,
};

--
2.20.1

2019-05-27 11:23:48

[permalink] [raw]

Subject: Re: [PATCHv6 0/4] omapdrm: DSI command mode panel support

* Tomi Valkeinen <[email protected]> [190527 10:51]:
> Hi,
>
> On 23/05/2019 23:07, Sebastian Reichel wrote:
> > Hi,
> >
> > Here is another round of the DSI command mode panel patchset
> > integrating the feedback from PATCHv5. The patches are based
> > on v5.2-rc1 tag. It does not contain the patches required for
> > OMAP3 support (it needs a workaround for a hardware bug) and
> > for automatic display rotation. They should get their own series,
> > once after everything has been moved to DRM panel API. I think
> > DRM panel conversion should happen _after_ this series, since
> > otherwise there is a high risk of bricking DSI support completely.
> > I already started a WIP branch for converting DSI to the DRM panel
> > API on top of this patchset.
>
> Looks good to me. For some reason I can't boot 5.2-rc2 (on x15) so I haven't
> been able to test yet. I'll pick the series up in any case, and I'll test it
> when I get the kernel booting.

Great good to have these merged finally :)

Hmm I wonder if some x15 models are affected by the SoC variant
changes queued in my fixes branch?

Regards,

Tony

2019-05-27 11:38:34

[permalink] [raw]

Subject: Re: [PATCHv6 0/4] omapdrm: DSI command mode panel support

Hi,

On 23/05/2019 23:07, Sebastian Reichel wrote:
> Hi,
>
> Here is another round of the DSI command mode panel patchset
> integrating the feedback from PATCHv5. The patches are based
> on v5.2-rc1 tag. It does not contain the patches required for
> OMAP3 support (it needs a workaround for a hardware bug) and
> for automatic display rotation. They should get their own series,
> once after everything has been moved to DRM panel API. I think
> DRM panel conversion should happen _after_ this series, since
> otherwise there is a high risk of bricking DSI support completely.
> I already started a WIP branch for converting DSI to the DRM panel
> API on top of this patchset.

Looks good to me. For some reason I can't boot 5.2-rc2 (on x15) so I
haven't been able to test yet. I'll pick the series up in any case, and
I'll test it when I get the kernel booting.

Tomi

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

2019-05-28 09:22:51

[permalink] [raw]

Subject: Re: [PATCHv6 0/4] omapdrm: DSI command mode panel support

On 27/05/2019 14:21, Tony Lindgren wrote:

>> Looks good to me. For some reason I can't boot 5.2-rc2 (on x15) so I haven't
>> been able to test yet. I'll pick the series up in any case, and I'll test it
>> when I get the kernel booting.
>
> Great good to have these merged finally :)
>
> Hmm I wonder if some x15 models are affected by the SoC variant
> changes queued in my fixes branch?

This is what I see with earlycon, on linux-omap fixes branch. I think this looks
similar to what I saw with dra76 _without_ the fixes.

[ 1.290771] Unable to handle kernel paging request at virtual address 5a5a5a5a
[ 1.298222] pgd = (ptrval)
[ 1.301002] [5a5a5a5a] *pgd=00000000
[ 1.304695] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[ 1.310158] Modules linked in:
[ 1.313300] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc1-00016-g43069e68f162-dirty #7
[ 1.321979] Hardware name: Generic DRA74X (Flattened Device Tree)
[ 1.328256] PC is at clk_hw_create_clk.part.33+0x8/0x94
[ 1.333632] LR is at sysc_notifier_call+0x98/0x138
[ 1.338558] pc : [<c0554118>] lr : [<c04fb514>] psr: 00000013
[ 1.345001] sp : eb8f7c78 ip : 5a5a5a5a fp : c0b3b538
[ 1.350374] r10: 00000001 r9 : 00000000 r8 : 00000000
[ 1.355746] r7 : eaab8340 r6 : eaabea10 r5 : cffea79c r4 : 00000000
[ 1.362459] r3 : cffea79c r2 : eaabc8c0 r1 : 5a5a5a5a r0 : eaabea10
[ 1.369174] Flags: nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
[ 1.376513] Control: 10c5387d Table: 8000406a DAC: 00000051
[ 1.382422] Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
[ 1.388599] Stack: (0xeb8f7c78 to 0xeb8f8000)
[ 1.393077] 7c60: 00000000 cffea79c
[ 1.401493] 7c80: eaabea10 c04fb514 c0e76c6c fffffffd 00000000 00000001 00000000 eaabea10
[ 1.409905] 7ca0: 00000001 c015cee8 ffffffff 00000001 eb8dfb80 eaabea10 c0e05148 00000000
[ 1.418319] 7cc0: 00000000 c0e2e4f0 00000000 c015d684 00000000 00000000 eaabea10 00000000
[ 1.426732] 7ce0: eb93c810 c05b97e4 00000000 00000000 00000000 0fee2f56 00000000 0fee2f56
[ 1.435143] 7d00: 00000000 eaabea00 00000000 eb93c810 efdbfd0c 00000000 00000001 c078edb0
[ 1.443557] 7d20: efdbfcc0 00000000 00000000 c0e05148 c0e2e390 c078ef88 eb93ca8c 00000000
[ 1.451972] 7d40: c0a39b64 eb93c810 eb93c88c eb93ca8c c0e05148 c091b0c4 eb93c810 c05c9aac
[ 1.460386] 7d60: c0f13ba4 60000013 c0a39b64 0fee2f56 c0a39b3c efdbfcc0 efdbfa3c c0a39b64
[ 1.468800] 7d80: c0e2e390 eb93c810 00000001 00000000 c0a39b3c c078f2b8 00000001 0fee2f56
[ 1.477213] 7da0: eaab8340 eaab8340 eb93c810 00000000 c0e05148 00000001 00000010 c04fc964
[ 1.485627] 7dc0: 00000000 c0349714 c0e2e500 eb93c810 00000001 efdbfa3c ea9dfd28 00000000
[ 1.494039] 7de0: 00000001 00000001 00000003 0fee2f56 00000001 eb93c810 00000000 c0e76c8c
[ 1.502451] 7e00: 00000000 00000000 c0e76c8c c0eb8f30 00000000 c05c00e8 eb93c810 c0f0f35c
[ 1.510865] 7e20: c0f0f360 00000000 00000000 c05bdc04 c0e76c8c eb93c810 c0eaf2a0 eb93c810
[ 1.519278] 7e40: c0e76c8c c0e76c8c c0e05148 ffffe000 c0d5b834 000000d8 c0eaf2a0 c05be0b4
[ 1.527692] 7e60: c0a39d00 a0000013 c0eaf2a0 eb93c810 00000000 c0e76c8c c0e05148 ffffe000
[ 1.536104] 7e80: c0d5b834 c05be450 00000000 c0e76c8c eb93c810 c05be4fc eb9380b4 c0e76c8c
[ 1.544516] 7ea0: c05be458 c05bbc24 c0eaf2a0 eb8dfb58 eb9380b4 0fee2f56 c0e76c8c c0e76c8c
[ 1.552928] 7ec0: ea9e4280 c0e85338 00000000 c05bcf68 c0b7aa98 c0e05148 c0d36b20 c0e76c8c
[ 1.561343] 7ee0: c0e05148 c0d36b20 00000000 c05bf0a0 c0eaf2a0 c0e05148 c0d36b20 c0102f88
[ 1.569757] 7f00: c0c4e10c 00000000 efd9a82f c015a900 00000000 eb8f7f14 c0d004a8 00000006
[ 1.578171] 7f20: 00000006 c0bc2e88 00000000 00000000 00000000 efd9a827 00000000 0fee2f56
[ 1.586583] 7f40: c0eaf2a0 c0d004a8 00000006 0fee2f56 c0d770ec 00000007 c0d5b854 c0eca080
[ 1.594997] 7f60: c0eca080 c0d01208 00000006 00000006 00000000 c0d004a8 c09145e0 00000000
[ 1.603411] 7f80: 00000000 00000000 c09145e0 00000000 00000000 00000000 00000000 00000000
[ 1.611823] 7fa0: 00000000 c09145e8 00000000 c01010e8 00000000 00000000 00000000 00000000
[ 1.620237] 7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 1.628651] 7fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[ 1.637074] [<c0554118>] (clk_hw_create_clk.part.33) from [<c04fb514>] (sysc_notifier_call+0x98/0x138)
[ 1.646656] [<c04fb514>] (sysc_notifier_call) from [<c015cee8>] (notifier_call_chain+0x2c/0xa0)
[ 1.655612] [<c015cee8>] (notifier_call_chain) from [<c015d684>] (blocking_notifier_call_chain+0x50/0x68)
[ 1.665461] [<c015d684>] (blocking_notifier_call_chain) from [<c05b97e4>] (device_add+0x3bc/0x628)
[ 1.674685] [<c05b97e4>] (device_add) from [<c078edb0>] (of_platform_device_create_pdata+0x90/0xbc)
[ 1.683997] [<c078edb0>] (of_platform_device_create_pdata) from [<c078ef88>] (of_platform_bus_create+0x1a0/0x328)
[ 1.694558] [<c078ef88>] (of_platform_bus_create) from [<c078f2b8>] (of_platform_populate+0x7c/0x108)
[ 1.704046] [<c078f2b8>] (of_platform_populate) from [<c04fc964>] (sysc_probe+0x9dc/0xf98)
[ 1.712552] [<c04fc964>] (sysc_probe) from [<c05c00e8>] (platform_drv_probe+0x48/0x98)
[ 1.720700] [<c05c00e8>] (platform_drv_probe) from [<c05bdc04>] (really_probe+0x100/0x40c)
[ 1.729205] [<c05bdc04>] (really_probe) from [<c05be0b4>] (driver_probe_device+0x6c/0x1b8)
[ 1.737709] [<c05be0b4>] (driver_probe_device) from [<c05be450>] (device_driver_attach+0x58/0x60)
[ 1.746836] [<c05be450>] (device_driver_attach) from [<c05be4fc>] (__driver_attach+0xa4/0x148)
[ 1.755700] [<c05be4fc>] (__driver_attach) from [<c05bbc24>] (bus_for_each_dev+0x70/0xb4)
[ 1.764116] [<c05bbc24>] (bus_for_each_dev) from [<c05bcf68>] (bus_add_driver+0x1a8/0x200)
[ 1.772620] [<c05bcf68>] (bus_add_driver) from [<c05bf0a0>] (driver_register+0x74/0x108)
[ 1.780948] [<c05bf0a0>] (driver_register) from [<c0102f88>] (do_one_initcall+0x48/0x29c)
[ 1.789365] [<c0102f88>] (do_one_initcall) from [<c0d01208>] (kernel_init_freeable+0x304/0x3d8)
[ 1.798317] [<c0d01208>] (kernel_init_freeable) from [<c09145e8>] (kernel_init+0x8/0x110)
[ 1.806732] [<c09145e8>] (kernel_init) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
[ 1.814516] Exception stack(0xeb8f7fb0 to 0xeb8f7ff8)
[ 1.819711] 7fa0: 00000000 00000000 00000000 00000000
[ 1.828124] 7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 1.836535] 7fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 1.843342] Code: 000010ac c0b85b48 e92d4070 e1a06000 (e5915000)
[ 1.849647] ---[ end trace ddabd37e7aa3d908 ]---
[ 1.854430] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
[ 1.862311] CPU1: stopping
[ 1.865098] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G D 5.2.0-rc1-00016-g43069e68f162-dirty #7
[ 1.875208] Hardware name: Generic DRA74X (Flattened Device Tree)
[ 1.881487] [<c011394c>] (unwind_backtrace) from [<c010da7c>] (show_stack+0x10/0x14)
[ 1.889460] [<c010da7c>] (show_stack) from [<c08fc9f4>] (dump_stack+0xa8/0xc4)
[ 1.896895] [<c08fc9f4>] (dump_stack) from [<c01119d8>] (handle_IPI+0x3ec/0x450)
[ 1.904510] [<c01119d8>] (handle_IPI) from [<c04f92b0>] (gic_handle_irq+0x94/0xa8)
[ 1.912302] [<c04f92b0>] (gic_handle_irq) from [<c0101aac>] (__irq_svc+0x6c/0xa8)
[ 1.919996] Exception stack(0xeb913f60 to 0xeb913fa8)
[ 1.925193] 3f60: 000006c0 00000000 efd90be0 c011e480 ffffe000 c0e05168 00000002 c0e051ac
[ 1.933608] 3f80: 00000000 c0e05148 00000000 c0e055b0 00000000 eb913fb0 c010a20c c010a210
[ 1.942017] 3fa0: 60000013 ffffffff
[ 1.945609] [<c0101aac>] (__irq_svc) from [<c010a210>] (arch_cpu_idle+0x30/0x3c)
[ 1.953222] [<c010a210>] (arch_cpu_idle) from [<c016cdf0>] (do_idle+0x1d8/0x2a8)
[ 1.960834] [<c016cdf0>] (do_idle) from [<c016d184>] (cpu_startup_entry+0x18/0x1c)
[ 1.968623] [<c016d184>] (cpu_startup_entry) from [<8010270c>] (0x8010270c)
[ 1.975793] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---

Tomi

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

2019-05-28 09:45:09

[permalink] [raw]

Subject: Re: [PATCHv6 0/4] omapdrm: DSI command mode panel support

Hi,

* Tomi Valkeinen <[email protected]> [190528 09:19]:
> On 27/05/2019 14:21, Tony Lindgren wrote:
>
> >> Looks good to me. For some reason I can't boot 5.2-rc2 (on x15) so I haven't
> >> been able to test yet. I'll pick the series up in any case, and I'll test it
> >> when I get the kernel booting.
> >
> > Great good to have these merged finally :)
> >
> > Hmm I wonder if some x15 models are affected by the SoC variant
> > changes queued in my fixes branch?
>
> This is what I see with earlycon, on linux-omap fixes branch. I think this looks
> similar to what I saw with dra76 _without_ the fixes.

OK sounds like we need to use some different SoC specific .dtsi file,
is this maybe x15 rev c?

You can detect which modules fail based on the module base address
for revision register seen with the following debug patch. Then
those need to be tagged with status = "disabled" at the module
level in the SoC specific dtsi file.

Regards,

Tony

8< --------------
diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -2069,6 +2069,8 @@ static int sysc_probe(struct platform_device *pdev)
struct sysc *ddata;
int error;

+ dev_info(&pdev->dev, "probing device\n");
+
ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL);
if (!ddata)
return -ENOMEM;

2019-05-28 09:53:17

by Keerthy

[permalink] [raw]

Subject: Re: [PATCHv6 0/4] omapdrm: DSI command mode panel support

On 28/05/19 3:09 PM, Tony Lindgren wrote:
> Hi,
>
> * Tomi Valkeinen <[email protected]> [190528 09:19]:
>> On 27/05/2019 14:21, Tony Lindgren wrote:
>>
>>>> Looks good to me. For some reason I can't boot 5.2-rc2 (on x15) so I haven't
>>>> been able to test yet. I'll pick the series up in any case, and I'll test it
>>>> when I get the kernel booting.
>>>
>>> Great good to have these merged finally :)
>>>
>>> Hmm I wonder if some x15 models are affected by the SoC variant
>>> changes queued in my fixes branch?
>>
>> This is what I see with earlycon, on linux-omap fixes branch. I think this looks
>> similar to what I saw with dra76 _without_ the fixes.
>
> OK sounds like we need to use some different SoC specific .dtsi file,
> is this maybe x15 rev c?
>
> You can detect which modules fail based on the module base address
> for revision register seen with the following debug patch. Then
> those need to be tagged with status = "disabled" at the module
> level in the SoC specific dtsi file.

Tomi,

My first suspect would be rtc.

diff --git a/arch/arm/boot/dts/am5728.dtsi b/arch/arm/boot/dts/am5728.dtsi
index 82e5427ef6a9..17b1b1b4db92 100644
--- a/arch/arm/boot/dts/am5728.dtsi
+++ b/arch/arm/boot/dts/am5728.dtsi
@@ -31,3 +31,7 @@
&atl_tm {
status = "disabled";
};
+
+&rtctarget {
+ status = "disabled";
+};

Regards,
Keerthy
>
> Regards,
>
> Tony
>
> 8< --------------
> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
> --- a/drivers/bus/ti-sysc.c
> +++ b/drivers/bus/ti-sysc.c
> @@ -2069,6 +2069,8 @@ static int sysc_probe(struct platform_device *pdev)
> struct sysc *ddata;
> int error;
>
> + dev_info(&pdev->dev, "probing device\n");
> +
> ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL);
> if (!ddata)
> return -ENOMEM;
>

2019-05-28 10:09:21

[permalink] [raw]

Subject: Re: [PATCHv6 0/4] omapdrm: DSI command mode panel support

On 28/05/2019 12:39, Tony Lindgren wrote:
> Hi,
>
> * Tomi Valkeinen <[email protected]> [190528 09:19]:
>> On 27/05/2019 14:21, Tony Lindgren wrote:
>>
>>>> Looks good to me. For some reason I can't boot 5.2-rc2 (on x15) so I haven't
>>>> been able to test yet. I'll pick the series up in any case, and I'll test it
>>>> when I get the kernel booting.
>>>
>>> Great good to have these merged finally :)
>>>
>>> Hmm I wonder if some x15 models are affected by the SoC variant
>>> changes queued in my fixes branch?
>>
>> This is what I see with earlycon, on linux-omap fixes branch. I think this looks
>> similar to what I saw with dra76 _without_ the fixes.
>
> OK sounds like we need to use some different SoC specific .dtsi file,
> is this maybe x15 rev c?
>
> You can detect which modules fail based on the module base address
> for revision register seen with the following debug patch. Then
> those need to be tagged with status = "disabled" at the module
> level in the SoC specific dtsi file.

[ 1.370609] ti-sysc 4ae20000.target-module: probing device

This change lets me boot. I don't know that's the correct place, though:

diff --git a/arch/arm/boot/dts/am5728.dtsi b/arch/arm/boot/dts/am5728.dtsi
index 82e5427ef6a9..c778f9a86b3a 100644
--- a/arch/arm/boot/dts/am5728.dtsi
+++ b/arch/arm/boot/dts/am5728.dtsi
@@ -31,3 +31,7 @@
&atl_tm {
status = "disabled";
};
+
+&timer12 {
+ status = "disabled";
+};

My board is x15 rev A3, attached to AM5 EVM. I've also attached my kernel
config.

Tomi

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Attachments:

.config (123.87 kB)

2019-05-28 10:35:36

[permalink] [raw]

Subject: Re: [PATCHv6 0/4] omapdrm: DSI command mode panel support

On 28/05/2019 13:18, Tony Lindgren wrote:

>> This change lets me boot. I don't know that's the correct place, though:
>>
>> diff --git a/arch/arm/boot/dts/am5728.dtsi b/arch/arm/boot/dts/am5728.dtsi
>> index 82e5427ef6a9..c778f9a86b3a 100644
>> --- a/arch/arm/boot/dts/am5728.dtsi
>> +++ b/arch/arm/boot/dts/am5728.dtsi
>> @@ -31,3 +31,7 @@
>> &atl_tm {
>> status = "disabled";
>> };
>> +
>> +&timer12 {
>> + status = "disabled";
>> +};
>
> OK we should disable it at the target-module level though. Interesting
> that reading the revision register works with the above, or maybe you
> still get some warning?

I don't see anything odd in the boot log with the above change. At least
no kernel WARNs, nor anything with grepping "timer", "err", or "warn".

I just verified with clean 5.2-rc2, that it doesn't boot, and with the
above change, boots.

>> My board is x15 rev A3, attached to AM5 EVM. I've also attached my kernel
>> config.
>
> Strange that this is not affecting other x15? I think timer12 would
> be blocked on HS devices though?

I don't know... I can't believe my x15 would be unique =). Can it be
something in the kernel config? u-boot?

Peter should have the same A3. Peter, can you try with my config?

Tomi

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

2019-05-28 10:49:59

[permalink] [raw]

Subject: Re: [PATCHv6 0/4] omapdrm: DSI command mode panel support

* Tomi Valkeinen <[email protected]> [190528 10:05]:
> On 28/05/2019 12:39, Tony Lindgren wrote:
> > Hi,
> >
> > * Tomi Valkeinen <[email protected]> [190528 09:19]:
> > > On 27/05/2019 14:21, Tony Lindgren wrote:
> > >
> > > > > Looks good to me. For some reason I can't boot 5.2-rc2 (on x15) so I haven't
> > > > > been able to test yet. I'll pick the series up in any case, and I'll test it
> > > > > when I get the kernel booting.
> > > >
> > > > Great good to have these merged finally :)
> > > >
> > > > Hmm I wonder if some x15 models are affected by the SoC variant
> > > > changes queued in my fixes branch?
> > >
> > > This is what I see with earlycon, on linux-omap fixes branch. I think this looks
> > > similar to what I saw with dra76 _without_ the fixes.
> >
> > OK sounds like we need to use some different SoC specific .dtsi file,
> > is this maybe x15 rev c?
> >
> > You can detect which modules fail based on the module base address
> > for revision register seen with the following debug patch. Then
> > those need to be tagged with status = "disabled" at the module
> > level in the SoC specific dtsi file.
>
> [ 1.370609] ti-sysc 4ae20000.target-module: probing device
>
> This change lets me boot. I don't know that's the correct place, though:
>
> diff --git a/arch/arm/boot/dts/am5728.dtsi b/arch/arm/boot/dts/am5728.dtsi
> index 82e5427ef6a9..c778f9a86b3a 100644
> --- a/arch/arm/boot/dts/am5728.dtsi
> +++ b/arch/arm/boot/dts/am5728.dtsi
> @@ -31,3 +31,7 @@
> &atl_tm {
> status = "disabled";
> };
> +
> +&timer12 {
> + status = "disabled";
> +};

OK we should disable it at the target-module level though. Interesting
that reading the revision register works with the above, or maybe you
still get some warning?

> My board is x15 rev A3, attached to AM5 EVM. I've also attached my kernel
> config.

Strange that this is not affecting other x15? I think timer12 would
be blocked on HS devices though?

Regards,

Tony

2019-05-28 11:30:16

[permalink] [raw]

Subject: Re: [PATCHv6 3/4] drm/omap: add framedone interrupt support

Hi Sebastian,

On 23/05/2019 23:07, Sebastian Reichel wrote:

> @@ -302,6 +328,30 @@ void omap_crtc_vblank_irq(struct drm_crtc *crtc)
> DBG("%s: apply done", omap_crtc->name);
> }
>
> +void omap_crtc_framedone_irq(struct drm_crtc *crtc, uint32_t irqstatus)
> +{
> + struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> +
> + if (!omap_crtc->framedone_handler) {
> + dev_warn(omap_crtc->base.dev->dev, "no framedone handler?");
> + return;
> + }

This triggers on normal displays.

FRAMEDONE is an interrupt we get when DISPC's output videoport is being
turned off. It's raised after the last frame has been finished (i.e. the
DISPC is truly done with that videoport).

We get it for both conventional displays (when the display is turned
off) and for DSI command mode (when a single frame has been sent), as in
both cases the videoport is disabled after the operation. For
conventional displays, you can think FRAMEDONE as the last vsync.

We also have special handling for FRAMEDONE in omap_crtc_set_enabled(),
which is used to get the drm driver to wait for FRAMEDONE when disabling
the display. I wonder if this separate framedone handling might somehow
conflict with that code. And/or should these be somehow combined.

Tomi

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

2019-05-29 07:08:12

[permalink] [raw]

Subject: Re: [PATCHv6 0/4] omapdrm: DSI command mode panel support

On 28/05/2019 13:18, Tony Lindgren wrote:

>> My board is x15 rev A3, attached to AM5 EVM. I've also attached my kernel
>> config.
>
> Strange that this is not affecting other x15? I think timer12 would
> be blocked on HS devices though?

Seems that the kernel config affects. omap2plus_defconfig boots ok.

Tomi

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

2019-05-29 08:12:13

[permalink] [raw]

Subject: Re: [PATCHv6 0/4] omapdrm: DSI command mode panel support

* Tomi Valkeinen <[email protected]> [190529 07:06]:
> On 28/05/2019 13:18, Tony Lindgren wrote:
>
> > > My board is x15 rev A3, attached to AM5 EVM. I've also attached my kernel
> > > config.
> >
> > Strange that this is not affecting other x15? I think timer12 would
> > be blocked on HS devices though?
>
> Seems that the kernel config affects. omap2plus_defconfig boots ok.

OK, this line in your oops:

Unable to handle kernel paging request at virtual address 5a5a5a5a

Probably means we hit some slab poison with DEBUG_SLAB set.
Looks like your config boots fine with DEBUG_SLAB disabled
for me.

As this only happens for timer12, I wonder if we're again
hitting some uncompress issue with corrupted dtb. Changing
u-boot ftdaddr higher up might possibly make it go away.
Or else there's a bug elsewhere :)

Regards,

Tony

2019-05-29 09:34:59

by Peter Ujfalusi

[permalink] [raw]

Subject: Re: [PATCHv6 0/4] omapdrm: DSI command mode panel support

On 28/05/2019 13.32, Tomi Valkeinen wrote:
> On 28/05/2019 13:18, Tony Lindgren wrote:
>> Strange that this is not affecting other x15? I think timer12 would
>> be blocked on HS devices though?
>
> I don't know... I can't believe my x15 would be unique =). Can it be
> something in the kernel config? u-boot?
>
> Peter should have the same A3. Peter, can you try with my config?

It did not boot with your config.
My config boots, I'm using SLUB.
Flipping CONFIG_SLUB_DEBUG_ON to y and the kernel does not boot.

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

2019-05-29 21:57:47

[permalink] [raw]

Subject: Re: [PATCHv6 3/4] drm/omap: add framedone interrupt support

Hi Tomi,

On Tue, May 28, 2019 at 01:19:01PM +0300, Tomi Valkeinen wrote:
> Hi Sebastian,
>
> On 23/05/2019 23:07, Sebastian Reichel wrote:
>
> > @@ -302,6 +328,30 @@ void omap_crtc_vblank_irq(struct drm_crtc *crtc)
> > DBG("%s: apply done", omap_crtc->name);
> > }
> > +void omap_crtc_framedone_irq(struct drm_crtc *crtc, uint32_t irqstatus)
> > +{
> > + struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> > +
> > + if (!omap_crtc->framedone_handler) {
> > + dev_warn(omap_crtc->base.dev->dev, "no framedone handler?");
> > + return;
> > + }
>
> This triggers on normal displays.
>
> FRAMEDONE is an interrupt we get when DISPC's output videoport is being
> turned off. It's raised after the last frame has been finished (i.e. the
> DISPC is truly done with that videoport).
>
> We get it for both conventional displays (when the display is turned off)
> and for DSI command mode (when a single frame has been sent), as in both
> cases the videoport is disabled after the operation. For conventional
> displays, you can think FRAMEDONE as the last vsync.

Ok, but it should only trigger when framedone irq is enabled. This
commit adds the required infrastructure, but does not call
omap_irq_enable_framedone() anywhere. The next commit enables it,
but only for manually updated displays.

> We also have special handling for FRAMEDONE in omap_crtc_set_enabled(),
> which is used to get the drm driver to wait for FRAMEDONE when disabling the
> display. I wonder if this separate framedone handling might somehow conflict
> with that code. And/or should these be somehow combined.

Oh sorry, I missed the part that omap_irq_wait_init() actually
enables the framedone irq. It should be enough to just drop the
warning (and the curly brackets) to keep existing behaviour. The
code exits early with the above warning for any existing code (since
that does not register a framedone handler). DSI on the other hand
does not reach the omap_irq_wait_init() part. Regarding combining
the logic: I don't think there is anything to combine right now.
It should be possible to simplify the logic after DSI has been
converted to drm_panel style, since this will move the update logic
for the screen content from the panel driver to DSI core.

TLDR: It's enough to remove the warning. Do you need a new
submission for this?

-- Sebastian

Attachments:

(No filename) (2.34 kB)
signature.asc (849.00 B)
Download all attachments

2019-05-30 05:48:02

[permalink] [raw]

Subject: Re: [PATCHv6 0/4] omapdrm: DSI command mode panel support

* Tony Lindgren <[email protected]> [190529 08:11]:
> * Tomi Valkeinen <[email protected]> [190529 07:06]:
> > On 28/05/2019 13:18, Tony Lindgren wrote:
> >
> > > > My board is x15 rev A3, attached to AM5 EVM. I've also attached my kernel
> > > > config.
> > >
> > > Strange that this is not affecting other x15? I think timer12 would
> > > be blocked on HS devices though?
> >
> > Seems that the kernel config affects. omap2plus_defconfig boots ok.
>
> OK, this line in your oops:
>
> Unable to handle kernel paging request at virtual address 5a5a5a5a
>
> Probably means we hit some slab poison with DEBUG_SLAB set.
> Looks like your config boots fine with DEBUG_SLAB disabled
> for me.
>
> As this only happens for timer12, I wonder if we're again
> hitting some uncompress issue with corrupted dtb. Changing
> u-boot ftdaddr higher up might possibly make it go away.
> Or else there's a bug elsewhere :)

Oh but CM_WKUPAON_TIMER12_CLKCTRL has no CLKSEL option unlike
CM_WKUPAON_TIMER1_CLKCTRL. Below is one part of the fix, but
it seems like we're missing handling somewhere as trying to
get a non-existing clock should just produce -ENODEV type error.

And the clksel should be just handled with assigned-clocks
in general, but I think we still need it there until we
have drivers/clocksource/ timer drivers updated to boot
using early_platform_device.

Regards,

Tony

8< ---------------
diff --git a/arch/arm/boot/dts/dra7-l4.dtsi b/arch/arm/boot/dts/dra7-l4.dtsi
--- a/arch/arm/boot/dts/dra7-l4.dtsi
+++ b/arch/arm/boot/dts/dra7-l4.dtsi
@@ -4450,8 +4450,6 @@
timer12: timer@0 {
compatible = "ti,omap5430-timer";
reg = <0x0 0x80>;
- clocks = <&wkupaon_clkctrl DRA7_WKUPAON_TIMER12_CLKCTRL 24>;
- clock-names = "fck";
interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
ti,timer-alwon;
ti,timer-secure;

2019-05-30 07:04:28

[permalink] [raw]

Subject: Re: [PATCHv6 0/4] omapdrm: DSI command mode panel support

* Tony Lindgren <[email protected]> [190530 05:47]:
> * Tony Lindgren <[email protected]> [190529 08:11]:
> > * Tomi Valkeinen <[email protected]> [190529 07:06]:
> > > On 28/05/2019 13:18, Tony Lindgren wrote:
> > >
> > > > > My board is x15 rev A3, attached to AM5 EVM. I've also attached my kernel
> > > > > config.
> > > >
> > > > Strange that this is not affecting other x15? I think timer12 would
> > > > be blocked on HS devices though?
> > >
> > > Seems that the kernel config affects. omap2plus_defconfig boots ok.
> >
> > OK, this line in your oops:
> >
> > Unable to handle kernel paging request at virtual address 5a5a5a5a
> >
> > Probably means we hit some slab poison with DEBUG_SLAB set.
> > Looks like your config boots fine with DEBUG_SLAB disabled
> > for me.
> >
> > As this only happens for timer12, I wonder if we're again
> > hitting some uncompress issue with corrupted dtb. Changing
> > u-boot ftdaddr higher up might possibly make it go away.
> > Or else there's a bug elsewhere :)
>
> Oh but CM_WKUPAON_TIMER12_CLKCTRL has no CLKSEL option unlike
> CM_WKUPAON_TIMER1_CLKCTRL. Below is one part of the fix, but
> it seems like we're missing handling somewhere as trying to
> get a non-existing clock should just produce -ENODEV type error.
>
> And the clksel should be just handled with assigned-clocks
> in general, but I think we still need it there until we
> have drivers/clocksource/ timer drivers updated to boot
> using early_platform_device.

OK found it, we have the clkctrl clock potentially return
uninitialized data. I posted two fixes for the issue:

[PATCH] clk: ti: clkctrl: Fix returning uninitialized data
[PATCH] ARM: dts: Drop bogus CLKSEL for timer12 on dra7

Regards,

Tony

2019-05-31 12:25:25