Re: [PATCH RFC v2 0/2] Disable planes on blanked CRTC and enable on unblank
On Fri, Nov 13, 2015 at 05:53:13PM +0200, Jyri Sarha wrote: > Since first RFC version: > - Added "drm/atomic: Track drm_plane's active state"-patch > > We would need something like this to get rid off OMAPDSS somewhat > messy runtime_resume code. How does this look, could this generic > solution be refined to be acceptable for mainline, or should we start > looking a local solution to omapdrm? One more comment besides what I've written in the older thread: You're not on latest drm-next, which has gained an active_only paramter to the plane commit helper. It might be best to discuss this topic on #dri-devel on freenode irc a bit. Cheers, Daniel > > Jyri Sarha (2): > drm/atomic: Track drm_plane's active state > drm/atomic: Disable planes on blanked CRTC and enable on unblank > > drivers/gpu/drm/drm_atomic_helper.c | 82 > + > drivers/gpu/drm/drm_plane_helper.c | 10 + > include/drm/drm_atomic_helper.h | 39 +- > include/drm/drm_crtc.h | 2 + > 4 files changed, 70 insertions(+), 63 deletions(-) > > -- > 1.9.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v4 3/8] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders
On Mon, Sep 28, 2015 at 12:26:28PM +0100, Russell King - ARM Linux wrote:
> On Mon, Sep 28, 2015 at 11:01:34AM +0200, Arnaud Pouliquen wrote:
> > few questions/remarks
> > BR,
> > Arnaud
> >
> > >+static void hdmi_codec_abort(struct device *dev)
> > >+{
> > >+struct hdmi_codec_priv *hcp = dev_get_drvdata(dev);
> > >+
> > >+dev_dbg(dev, "%s()\n", __func__);
> > >+
> > >+mutex_lock(>current_stream_lock);
> > >+if (hcp->current_stream && hcp->current_stream->runtime &&
> > >+snd_pcm_running(hcp->current_stream)) {
> > >+dev_info(dev, "HDMI audio playback aborted\n");
> > >+snd_pcm_stream_lock_irq(hcp->current_stream);
> > >+snd_pcm_stop(hcp->current_stream, SNDRV_PCM_STATE_DISCONNECTED);
> > >+snd_pcm_stream_unlock_irq(hcp->current_stream);
> > >+}
> > >+mutex_unlock(>current_stream_lock);
> > >+}
> > Does driver should stop the stream in case of unplug?
> > This could generate unexpected behavior at application level.
> > Perhaps should be better if this part is managed in DRM driver. if HDMI
> > master, I2S bus should be maintained ON to consume the audio stream until
> > application action.
>
> If it does, that's really horrid.
Atm the rule for display outputs is that nothing gets yanked until
userspace approves, since otherwise compositors get stuck (or fall over
with an unexpected -EINVAL from the kernel). The exception is DP MST
because the current implementation is a complete hack for DP MST sink
lifetimes and that's why we need to synchronously nuke them (which means
shutting down everything). I'm surprised not a hole lot more people
complain about this ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/21] drm/omap: handle mismatching color format and buffer width
On Thu, Feb 26, 2015 at 03:20:17PM +0200, Tomi Valkeinen wrote:
omapdrm doesn't check if the width of the framebuffer and the color
format's bits-per-pixel match.
For example, using a display with a width of 1280, and a buffer
allocated with using 32 bits per pixel (i.e. 1280*4 = 5120 bytes), with
a 24 bits per pixel color format, leads to the following mismatch:
5120/3 = 1706.666... bytes. This causes bad colors and a tilt on the
screen.
Add a check into omapdrm to return an error if the user tries to use
such a combination.
Signed-off-by: Tomi Valkeinen tomi.valkei...@ti.com
---
drivers/gpu/drm/omapdrm/omap_fb.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c
b/drivers/gpu/drm/omapdrm/omap_fb.c
index 2975096abdf5..bf98580223d0 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -463,6 +463,14 @@ struct drm_framebuffer *omap_framebuffer_init(struct
drm_device *dev,
goto fail;
}
+ if (mode_cmd-width % format-planes[i].stride_bpp != 0) {
width is in pixels. No idea what you're trying to check here, but this
probably isn't it.
Also drm checks that things fit into the specified pitch (which is in
bytes), see the pichtes[i] width * cpp check in framebuffer_check.
Cheers, Daniel
+ dev_err(dev-dev,
+ buffer width (%d) is not a multiple of pixel
width (%d)\n,
+ mode_cmd-width, format-planes[i].stride_bpp);
+ ret = -EINVAL;
+ goto fail;
+ }
+
size = pitch * mode_cmd-height / format-planes[i].sub_y;
if (size (omap_gem_mmap_size(bos[i]) - mode_cmd-offsets[i]))
{
--
2.3.0
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/21] drm/omap: handle mismatching color format and buffer width
On Fri, Feb 27, 2015 at 02:40:20PM +, Daniel Stone wrote:
On 27 February 2015 at 13:01, Daniel Vetter dan...@ffwll.ch wrote:
On Thu, Feb 26, 2015 at 03:20:17PM +0200, Tomi Valkeinen wrote:
omapdrm doesn't check if the width of the framebuffer and the color
diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c
b/drivers/gpu/drm/omapdrm/omap_fb.c
index 2975096abdf5..bf98580223d0 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -463,6 +463,14 @@ struct drm_framebuffer *omap_framebuffer_init(struct
drm_device *dev,
goto fail;
}
+ if (mode_cmd-width % format-planes[i].stride_bpp != 0) {
width is in pixels. No idea what you're trying to check here, but this
probably isn't it.
stride_bpp is very misnamed: it is the bits per pixel for that plane,
and not stride at all. I think the check should in fact be be (pitch %
format-planes[i].stride_bpp), which would achieve the desired result,
i.e. that the stride can be expressed as an integer number of pixels.
I meant that mode_cmd-width is in pixels and so totally not what you want
to check here. It probably should be mode_cmd-pitches[i].
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/21] drm/omap: check CRTC color format earlier
On Thu, Feb 26, 2015 at 03:20:14PM +0200, Tomi Valkeinen wrote:
When setting a color format to a DRM plane, the DRM core checks whether
the format is supported by the HW. However, it seems that when setting
the color format of a CRTC (i.e. a root plane), there's no checking
done. This causes omapdrm to configure omapdss with the bad color
format, which omapdss then rejects.
While the above is ok, the error message is a bit confusing, and the
configuring of omapdss happens asynchronously from the ioctl that does
the color format change.
This patch adds a color format check to omap_plane_mode_set() which
causes the ioctl to return an error for invalid color format. This means
that the userspace will get to know of the wrong setting, instead of the
current behavior where the error is not seen by the userspace.
Signed-off-by: Tomi Valkeinen tomi.valkei...@ti.com
---
drivers/gpu/drm/omapdrm/omap_plane.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
b/drivers/gpu/drm/omapdrm/omap_plane.c
index 1f4f2b866379..bedd6f7af0f1 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -207,6 +207,24 @@ int omap_plane_mode_set(struct drm_plane *plane,
{
struct omap_plane *omap_plane = to_omap_plane(plane);
struct omap_drm_window *win = omap_plane-win;
+ int i;
+
+ /*
+ * Check whether this plane supports the fb pixel format.
+ * I don't think this should really be needed, but it looks like the
+ * drm core only checks the format for planes, not for the crtc. So
+ * when setting the format for crtc, without this check we would
+ * get an error later when trying to program the color format into the
+ * HW.
+ */
I think we should add a format check to the setcrtc ioctl if crtc-primary
is set. Atm the code is in __setplane_internal but could easily be shared
- there's already a copy in drm_atomi.c.
Omapdrm wouldn't benefit from this yet since it doesn't support universal
planes. But adding universal planes should be on your todo anyway ;-)
-Daniel
+ for (i = 0; i plane-format_count; i++)
+ if (fb-pixel_format == plane-format_types[i])
+ break;
+ if (i == plane-format_count) {
+ DBG(Invalid pixel format %s,
+ drm_get_format_name(fb-pixel_format));
+ return -EINVAL;
+ }
win-crtc_x = crtc_x;
win-crtc_y = crtc_y;
--
2.3.0
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RESEND 0/9] drm: tilcdc driver fixes
On Sat, Jun 28, 2014 at 06:51:15AM -0400, Rob Clark wrote: On Fri, Jun 27, 2014 at 6:08 PM, Darren Etheridge detheri...@ti.com wrote: Guido, On 06/17/2014 09:17 AM, Guido MartÃnez wrote: The tilcdc driver could be compiled as a module, but was severely broken and could not be used as such. This patchset attempts to fix the issues preventing a proper load/unload of the module. Issues included dangling sysfs nodes, dangling devices, memory leaks and a double kfree. It now seems to be working ok. We have tested this by loading and unloading the driver repeteadly, with both panel and slave connectors and found no flaws. There is still one warning left on tilcdc_crtc_destroy, caused by destroying the connector while still in an ON status. We don't know why this happens or why it's an issue, so we did not fix it. Yes I see what you mean, it triggers the WARN_ON in tilcdc_crtc_destroy because DRM_MODE_DPMS_ON is still set. This WARN_ON does make some sense because DPMS_OFF would have the effect of turning off clocks and putting the monitor to sleep which seems logical considering we have torn down the display. Adding a tilcdc_crtc_dpms(DPMS_OFF) right before the WARN_ON confirms this, but it seems strange that this hasn't happened automatically (+ Russell doesn't need to do it in his Armada driver) - so I suspect there is a better way. tbh, I'm not entirely sure offhand why drm_mode_config_cleanup() doesn't remove the fb's first (which should have the effect of shutting down any lit crtc/encoder/connector).. that would seem like the sensible way to shut down.. All userspace fbs should be cleared already before going into the driver unload. Which only leaves you with driver internal fbs (usually just one for fbdev emulation). It's the driver's job to clean that up explicitly. Then you can call mode_config_cleanup and the WARN_ON in there is a really nice space leak check. If we'd unconditionally clean up all fbs we'd have trouble with driver-private embedded fbs and their refcounting and would loose the space leak check. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm/omap: Take a fb reference in omap_plane_update()
On Tue, Apr 09, 2013 at 05:56:02PM +0530, Archit Taneja wrote:
When userspace calls SET_PLANE ioctl, drm core takes a reference of the fb and
passes control to the update_plane op defined by the drm driver.
In omapdrm, we have a worker thread which queues framebuffers objects received
from update_plane and displays them at the appropriate time.
It is possible that the framebuffer is destoryed by userspace between the time
of calling the ioctl and apply-worker being scheduled. If this happens, the
apply-worker holds a pointer to a framebuffer which is already destroyed.
Take an extra refernece/unreference of the fb in omap_plane_update() to
prevent
this from happening. A reference is taken of the fb passed to update_plane(),
the previous framebuffer (held by plane-fb) is unreferenced. This will
prevent
drm from destroying the framebuffer till the time it's unreferenced by the
apply-worker.
This is in addition to the exisitng reference/unreference in update_pin(),
which is taken for the scanout of the plane's current framebuffer, and an
unreference the previous framebuffer.
Signed-off-by: Archit Taneja arc...@ti.com
Cc: Rob Clark robdcl...@gmail.com
---
drivers/gpu/drm/omapdrm/omap_plane.c |6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
b/drivers/gpu/drm/omapdrm/omap_plane.c
index 2882cda..8d225d7 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -247,6 +247,12 @@ static int omap_plane_update(struct drm_plane *plane,
{
struct omap_plane *omap_plane = to_omap_plane(plane);
omap_plane-enabled = true;
+
+ if (plane-fb)
+ drm_framebuffer_unreference(plane-fb);
Shouldn't the unref only happen once the flip has completed? Otherwise we
might free the memory which is still being scanned out and put some other
crap there.
Note that I've been too lazy to read the code, but I expected the unref of
the old one to happen after the ref-taking for the new one, with a vblank
wait/delay in between ... Might be that I'm completely off ;-)
-Daniel
+
+ drm_framebuffer_reference(fb);
+
return omap_plane_mode_set(plane, crtc, fb,
crtc_x, crtc_y, crtc_w, crtc_h,
src_x, src_y, src_w, src_h,
--
1.7.10.4
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm/omap: Take a fb reference in omap_plane_update()
On Tue, Apr 9, 2013 at 3:17 PM, Rob Clark robdcl...@gmail.com wrote:
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
b/drivers/gpu/drm/omapdrm/omap_plane.c
index 2882cda..8d225d7 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -247,6 +247,12 @@ static int omap_plane_update(struct drm_plane *plane,
{
struct omap_plane *omap_plane = to_omap_plane(plane);
omap_plane-enabled = true;
+
+ if (plane-fb)
+ drm_framebuffer_unreference(plane-fb);
Shouldn't the unref only happen once the flip has completed? Otherwise we
might free the memory which is still being scanned out and put some other
crap there.
yup, there is a 2nd ref grabbed when we start scanout and dropped when
we finish scanout.. so that part was already covered. What wasn't
covered before was the time between the ioctl and the worker thread
(which was grabbing/dropping the scanout ref)
Ah, I see. And the ordering doesn't seem to matter here since it's all
protected by locks (against races with the worker thread) anyway.
Thanks for the explanation.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] drm/i2c: nxp-tda998x (v2)
On Tue, Jan 22, 2013 at 04:36:23PM -0600, Rob Clark wrote:
Driver for the NXP TDA998X i2c hdmi encoder slave.
v1: original
v2: fix npix/nline programming
Signed-off-by: Rob Clark robdcl...@gmail.com
Just one bikeshed, otherwise
Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
[cut]
+static void
+reg_set(struct drm_encoder *encoder, uint16_t reg, uint8_t val)
+{
+ reg_write(encoder, reg, reg_read(encoder, reg) | val);
+}
+
+static void
+reg_clear(struct drm_encoder *encoder, uint16_t reg, uint8_t val)
+{
+ reg_write(encoder, reg, reg_read(encoder, reg) ~val);
+}
What about drivers/base/regmap? I haven't looked to closely yet and never
used it in code, but there's a presentation [1] and it sounds like it
provides some nice (and more important standardized) helper stuff for
debug, tracing, ...
Since encoder slave drivers tend to be utterly boring register bashing and
we expect tons of time, I think high levels of standardization would be
really useful. Care to look into this a bit?
Cheers, Daniel
1: http://free-electrons.com/blog/fosdem2012-videos/
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] drm/tilcdc: add encoder slave
On Tue, Jan 22, 2013 at 04:36:24PM -0600, Rob Clark wrote:
Add output panel driver for i2c encoder slaves.
Signed-off-by: Rob Clark robdcl...@gmail.com
A few questions below, otherwise
Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/tilcdc/Kconfig| 12 ++
drivers/gpu/drm/tilcdc/Makefile | 1 +
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 5 +-
drivers/gpu/drm/tilcdc/tilcdc_slave.c | 380
++
drivers/gpu/drm/tilcdc/tilcdc_slave.h | 26 +++
5 files changed, 423 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/tilcdc/tilcdc_slave.c
create mode 100644 drivers/gpu/drm/tilcdc/tilcdc_slave.h
diff --git a/drivers/gpu/drm/tilcdc/Kconfig b/drivers/gpu/drm/tilcdc/Kconfig
index ee9b592..99beca2 100644
--- a/drivers/gpu/drm/tilcdc/Kconfig
+++ b/drivers/gpu/drm/tilcdc/Kconfig
@@ -8,3 +8,15 @@ config DRM_TILCDC
Choose this option if you have an TI SoC with LCDC display
controller, for example AM33xx in beagle-bone, DA8xx, or
OMAP-L1xx. This driver replaces the FB_DA8XX fbdev driver.
+
+menu I2C encoder or helper chips
+ depends on DRM DRM_KMS_HELPER I2C
+
+config DRM_I2C_NXP_TDA998X
+ tristate NXP Semiconductors TDA998X HDMI encoder
+ default m if DRM_TILCDC
+ help
+ Support for NXP Semiconductors TDA998X HDMI encoders.
+
+endmenu
+
Shouldn't that hunk be in patch 2?
diff --git a/drivers/gpu/drm/tilcdc/Makefile b/drivers/gpu/drm/tilcdc/Makefile
index 1359cc2..aa9097e 100644
--- a/drivers/gpu/drm/tilcdc/Makefile
+++ b/drivers/gpu/drm/tilcdc/Makefile
@@ -3,6 +3,7 @@ ccflags-y := -Iinclude/drm -Werror
tilcdc-y := \
tilcdc_crtc.o \
tilcdc_tfp410.o \
+ tilcdc_slave.o \
tilcdc_drv.o
obj-$(CONFIG_DRM_TILCDC) += tilcdc.o
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index cf1fddc..ca76dbe 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -20,6 +20,7 @@
#include tilcdc_drv.h
#include tilcdc_regs.h
#include tilcdc_tfp410.h
+#include tilcdc_slave.h
#include drm_fb_helper.h
@@ -587,6 +588,7 @@ static int __init tilcdc_drm_init(void)
{
DBG(init);
tilcdc_tfp410_init();
+ tilcdc_slave_init();
return platform_driver_register(tilcdc_platform_driver);
}
@@ -594,10 +596,11 @@ static void __exit tilcdc_drm_fini(void)
{
DBG(fini);
tilcdc_tfp410_fini();
+ tilcdc_slave_fini();
platform_driver_unregister(tilcdc_platform_driver);
}
-module_init(tilcdc_drm_init);
+late_initcall(tilcdc_drm_init);
module_exit(tilcdc_drm_fini);
MODULE_AUTHOR(Rob Clark robdcl...@gmail.com);
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave.c
b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
new file mode 100644
index 000..b6f3e63
--- /dev/null
+++ b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
@@ -0,0 +1,380 @@
+/*
+ * Copyright (C) 2012 Texas Instruments
+ * Author: Rob Clark robdcl...@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
with
+ * this program. If not, see http://www.gnu.org/licenses/.
+ */
+
+#include linux/i2c.h
+#include linux/of_i2c.h
+#include linux/pinctrl/pinmux.h
+#include linux/pinctrl/consumer.h
+#include drm/drm_encoder_slave.h
+
+#include tilcdc_drv.h
+
+struct slave_module {
+ struct tilcdc_module base;
+ struct i2c_adapter *i2c;
+};
+#define to_slave_module(x) container_of(x, struct slave_module, base)
+
+static const struct tilcdc_panel_info slave_info = {
+ .min_bpp= 16,
+ .max_bpp= 16,
+ .bpp= 16,
+ .ac_bias= 255,
+ .ac_bias_intrpt = 0,
+ .dma_burst_sz = 16,
+ .fdd= 0x80,
+ .tft_alt_mode = 0,
+ .stn_565_mode = 0,
+ .mono_8bit_mode = 0,
+ .sync_edge = 0,
+ .sync_ctrl = 1,
+ .raster_order = 0,
+};
+
+
+/*
+ * Encoder:
+ */
+
+struct slave_encoder {
+ struct drm_encoder_slave base;
+ struct slave_module *mod;
+};
+#define to_slave_encoder(x) container_of(to_encoder_slave(x), struct
slave_encoder, base)
Since you have a 1:1:1 relationship between module/drm_encoder
Re: [PATCH 3/4] drm/tilcdc: add encoder slave
On Tue, Jan 22, 2013 at 04:36:24PM -0600, Rob Clark wrote:
Add output panel driver for i2c encoder slaves.
Signed-off-by: Rob Clark robdcl...@gmail.com
Found some more stuff ...
[cut]
+static const struct drm_encoder_helper_funcs slave_encoder_helper_funcs = {
+ .dpms = drm_i2c_encoder_dpms,
+ .mode_fixup = drm_i2c_encoder_mode_fixup,
+ .prepare= slave_encoder_prepare,
+ .commit = drm_i2c_encoder_commit,
+ .mode_set = drm_i2c_encoder_mode_set,
+ .save = drm_i2c_encoder_save,
+ .restore= drm_i2c_encoder_restore,
+};
I couldn't find these wrappers anywhere ...
+
+static const struct i2c_board_info info = {
+ I2C_BOARD_INFO(tda998x, 0x70)
+};
Shouldn't there be some of/devicetree thing to tell us which one to load?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] drm/tilcdc: add support for LCD panels (v4)
On Tue, Jan 22, 2013 at 04:36:25PM -0600, Rob Clark wrote:
Add an output panel driver for LCD panels. Tested with LCD3 cape on
beaglebone.
v1: original
v2: s/of_find_node_by_name()/of_get_child_by_name()/ from Pantelis
Antoniou
v3: add backlight support
v4: rebase to latest of video timing helpers
Signed-off-by: Rob Clark robdcl...@gmail.com
So given that I'm utterly lacking clue about all things of (which seems to
be where all the magic in this patch lies) I'm just gonna ask a few funny
questions.
[cut]
+static int panel_connector_get_modes(struct drm_connector *connector)
+{
+ struct drm_device *dev = connector-dev;
+ struct panel_connector *panel_connector = to_panel_connector(connector);
+ struct display_timings *timings = panel_connector-mod-timings;
+ int i;
+
+ for (i = 0; i timings-num_timings; i++) {
+ struct drm_display_mode *mode = drm_mode_create(dev);
+ struct videomode vm;
+
+ if (videomode_from_timing(timings, vm, i))
+ break;
+
+ drm_display_mode_from_videomode(vm, mode);
Why do we need to jump through the intermediate videomode thing here? Is
that a deficiency of the of/videomode stuff?
[cut]
+ ret |= of_property_read_u32(info_np, ac-bias, info-ac_bias);
+ ret |= of_property_read_u32(info_np, ac-bias-intrpt,
info-ac_bias_intrpt);
+ ret |= of_property_read_u32(info_np, dma-burst-sz,
info-dma_burst_sz);
+ ret |= of_property_read_u32(info_np, bpp, info-bpp);
+ ret |= of_property_read_u32(info_np, fdd, info-fdd);
+ ret |= of_property_read_u32(info_np, sync-edge, info-sync_edge);
+ ret |= of_property_read_u32(info_np, sync-ctrl, info-sync_ctrl);
+ ret |= of_property_read_u32(info_np, raster-order,
info-raster_order);
+ ret |= of_property_read_u32(info_np, fifo-th, info-fifo_th);
Shouldn't these values all be documented somewhere in the devictree docs?
Or are they somewhat standardized?
+
+ /* optional: */
+ info-tft_alt_mode = of_property_read_bool(info_np,
tft-alt-mode);
+ info-stn_565_mode = of_property_read_bool(info_np,
stn-565-mode);
+ info-mono_8bit_mode= of_property_read_bool(info_np,
mono-8bit-mode);
+ info-invert_pxl_clk= of_property_read_bool(info_np,
invert-pxl-clk);
+
+ if (of_property_read_u32(info_np, max-bpp, info-max_bpp))
+ info-max_bpp = info-bpp;
+ if (of_property_read_u32(info_np, min-bpp, info-min_bpp))
+ info-min_bpp = info-bpp;
+
+ if (ret) {
+ pr_err(%s: error reading panel-info properties\n, __func__);
+ kfree(info);
+ return NULL;
+ }
+
+ return info;
+}
+
+static struct of_device_id panel_of_match[];
+
+static int panel_probe(struct platform_device *pdev)
+{
+ struct device_node *node = pdev-dev.of_node;
+ struct panel_module *panel_mod;
+ struct tilcdc_module *mod;
+ struct pinctrl *pinctrl;
+ int ret = -EINVAL;
+
+
+ /* bail out early if no DT data: */
+ if (!node) {
+ dev_err(pdev-dev, device-tree data is missing\n);
+ return -ENXIO;
+ }
+
+ panel_mod = kzalloc(sizeof(*panel_mod), GFP_KERNEL);
+ if (!panel_mod)
+ return -ENOMEM;
+
+ mod = panel_mod-base;
+
+ tilcdc_module_init(mod, panel, panel_module_ops);
+
+ pinctrl = devm_pinctrl_get_select_default(pdev-dev);
+ if (IS_ERR(pinctrl))
+ dev_warn(pdev-dev, pins are not configured\n);
+
+
+ panel_mod-timings = of_get_display_timings(node);
+ if (!panel_mod-timings) {
+ dev_err(pdev-dev, could not get panel timings\n);
+ goto fail;
+ }
+
+ panel_mod-info = of_get_panel_info(node);
+ if (!panel_mod-info) {
+ dev_err(pdev-dev, could not get panel info\n);
+ goto fail;
+ }
+
+ panel_mod-backlight = of_find_backlight_by_node(node);
If this _really_ works that easily, I'll have of-envy for the rest of my
life :(
/me hates the real-world abomination called Intel backlight handling
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] drm/tilcdc: add TI LCD Controller DRM driver (v3)
On Tue, Jan 22, 2013 at 04:36:22PM -0600, Rob Clark wrote:
A simple DRM/KMS driver for the TI LCD Controller found in various
smaller TI parts (AM33xx, OMAPL138, etc). This driver uses the
CMA helpers. Currently only the TFP410 DVI encoder is supported
(tested with beaglebone + DVI cape). There are also various LCD
displays, for which support can be added (as I get hw to test on),
and an external i2c HDMI encoder found on some boards.
The display controller supports a single CRTC. And the encoder+
connector are split out into sub-devices. Depending on which LCD
or external encoder is actually present, the appropriate output
module(s) will be loaded.
v1: original
v2: fix fb refcnting and few other cleanups
v3: get +/- vsync/hsync from timings rather than panel-info, add
option DT max-bandwidth field so driver doesn't attempt to
pick a display mode with too high memory bandwidth, and other
small cleanups
Signed-off-by: Rob Clark robdcl...@gmail.com
Ok, read through it, looks nice. No idea whether this should use the panel
stuff, but since no-one else does who cares. A few nits and questions
below. Was a nice reading to learn about kfifo and the runtime power
stuff.
Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/Kconfig| 2 +
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/tilcdc/Kconfig | 10 +
drivers/gpu/drm/tilcdc/Makefile| 8 +
drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 597
drivers/gpu/drm/tilcdc/tilcdc_drv.c| 605
+
drivers/gpu/drm/tilcdc/tilcdc_drv.h| 159 +
drivers/gpu/drm/tilcdc/tilcdc_regs.h | 154 +
drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 423 +++
drivers/gpu/drm/tilcdc/tilcdc_tfp410.h | 26 ++
10 files changed, 1985 insertions(+)
create mode 100644 drivers/gpu/drm/tilcdc/Kconfig
create mode 100644 drivers/gpu/drm/tilcdc/Makefile
create mode 100644 drivers/gpu/drm/tilcdc/tilcdc_crtc.c
create mode 100644 drivers/gpu/drm/tilcdc/tilcdc_drv.c
create mode 100644 drivers/gpu/drm/tilcdc/tilcdc_drv.h
create mode 100644 drivers/gpu/drm/tilcdc/tilcdc_regs.h
create mode 100644 drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
create mode 100644 drivers/gpu/drm/tilcdc/tilcdc_tfp410.h
[cut]
+struct tilcdc_crtc {
+ struct drm_crtc base;
+
+ const struct tilcdc_panel_info *info;
+ uint32_t dirty;
+ dma_addr_t start, end;
+ struct drm_pending_vblank_event *event;
+ int dpms;
+ wait_queue_head_t frame_done_wq;
+ bool frame_done;
+
+ /* fb currently set to scanout 0/1: */
+ struct drm_framebuffer *scanout[2];
+
+ /* for deferred fb unref's: */
+ DECLARE_KFIFO_PTR(unref_fifo, struct drm_framebuffer *);
+ struct work_struct work;
+};
+#define to_tilcdc_crtc(x) container_of(x, struct tilcdc_crtc, base)
+
+static void unref_worker(struct work_struct *work)
+{
+ struct tilcdc_crtc *tilcdc_crtc = container_of(work, struct
tilcdc_crtc, work);
+ struct drm_device *dev = tilcdc_crtc-base.dev;
+ struct drm_framebuffer *fb;
+
+ mutex_lock(dev-mode_config.mutex);
+ while (kfifo_get(tilcdc_crtc-unref_fifo, fb))
+ drm_framebuffer_unreference(fb);
+ mutex_unlock(dev-mode_config.mutex);
Hm, just learned about the kfifo api. It looks like the locking here still
works even with the new modeset locking, since kfifo explicitly allows
concurrent readers and writers without locking, as long as there's only on
of each kind. But maybe switch over to crtc-mutex to make things less
tricky.
Also, kfifo seems to have a new api which allows embedding of the kfifo
thing and needs to be used with kfifo_in/out.
[cut]
+static void update_scanout(struct drm_crtc *crtc)
+{
+ struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
+ struct drm_device *dev = crtc-dev;
+ struct drm_framebuffer *fb = crtc-fb;
+ struct drm_gem_cma_object *gem;
+ unsigned int depth, bpp;
+
+ drm_fb_get_bpp_depth(fb-pixel_format, depth, bpp);
+ gem = drm_fb_cma_get_gem_obj(fb, 0);
+
+ tilcdc_crtc-start = gem-paddr + fb-offsets[0] +
+ (crtc-y * fb-pitches[0]) + (crtc-x * bpp/8);
+
+ tilcdc_crtc-end = tilcdc_crtc-start +
+ (crtc-mode.vdisplay * fb-pitches[0]);
+
+ if (tilcdc_crtc-dpms == DRM_MODE_DPMS_ON) {
+ /* already enabled, so just mark the frames that need
+ * updating and they will be updated on vblank:
+ */
+ tilcdc_crtc-dirty |= LCDC_END_OF_FRAME0 | LCDC_END_OF_FRAME1;
+ drm_vblank_get(dev, 0);
+ } else {
+ /* not enabled yet, so update registers immediately: */
+ set_scanout(crtc, 0);
+ set_scanout(crtc, 1);
At least on intel we disallow pageflips on disabled crtcs since
drm_vblank_get
Re: [RFC] drm/lcdc: add TI LCD Controller DRM driver
On Fri, Dec 14, 2012 at 1:04 AM, Rob Clark robdcl...@gmail.com wrote:
+static int lcdc_crtc_page_flip(struct drm_crtc *crtc,
+ struct drm_framebuffer *fb,
+ struct drm_pending_vblank_event *event)
+{
+ struct lcdc_crtc *lcdc_crtc = to_lcdc_crtc(crtc);
+ struct drm_device *dev = crtc-dev;
+
+ if (lcdc_crtc-event) {
+ dev_err(dev-dev, already pending page flip!\n);
+ return -EBUSY;
+ }
+
+ // TODO we should hold a ref to the fb somewhere..
Note that with the current fb refcounting nothing prevents you from
fixing this. The ugly problems I've had to solve for the locking
rework are all due to the drm core (i.e. setcrtc/pageflip/...) ioctl
functions assuming that an fb can't suddenly disappear while holding
the mode_config lock. Since I wanted to remove that requirement I've
had to changed the refcounting in the drm core functions.
But drivers can already extend the lifetime of an fb simply by
grabbing a reference (as long as they grab that reference while
holding the struct mutex all the time between fb lookup and grabbing
the reference). And it will continue to work the same with the new
locking scheme.
-Daniel
+ crtc-fb = fb;
+ lcdc_crtc-event = event;
+ update_scanout(crtc);
+
+ return 0;
+}
+
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
