[patch 2/2 v2] drm/exynos: fix a warning message
The "ret = regmap_write()" assignment was missing so this error message
is never printed.
Signed-off-by: Dan Carpenter
---
v2: in v1 I just deleted the error message
diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c
b/drivers/gpu/drm/exynos/exynos_drm_mic.c
index 9869d70..ea2ea17 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
@@ -129,7 +129,7 @@ static void mic_set_path(struct exynos_mic *mic, bool
enable)
} else
val &= ~(MIC0_RGB_MUX | MIC0_I80_MUX | MIC0_ON_MUX);
- regmap_write(mic->sysreg, DSD_CFG_MUX, val);
+ ret = regmap_write(mic->sysreg, DSD_CFG_MUX, val);
if (ret)
DRM_ERROR("mic: Failed to read system register\n");
}
[patch 2/2] drm/exynos: mic: remove some dead code
On Fri, Mar 25, 2016 at 05:51:20PM +0900, Inki Dae wrote:
> Hi Dan,
>
> 2016ë
03ì 17ì¼ 19:39ì Dan Carpenter ì´(ê°) ì´ ê¸:
> > We know "ret" is zero and the test makes static checkers complain so
> > let's delete this printk.
> >
> > Signed-off-by: Dan Carpenter
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c
> > b/drivers/gpu/drm/exynos/exynos_drm_mic.c
> > index 890c9b1..12db353 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
> > @@ -130,8 +130,6 @@ static void mic_set_path(struct exynos_mic *mic, bool
> > enable)
> > val &= ~(MIC0_RGB_MUX | MIC0_I80_MUX | MIC0_ON_MUX);
> >
> > regmap_write(mic->sysreg, DSD_CFG_MUX, val);
> > - if (ret)
> > - DRM_ERROR("mic: Failed to read system register\n");
>
> I think we missed to keep return value from regmap_write function,
> ret = regmap_write(mic->sysreg, );
> if (ret)
> ...
Yeah. You're right.
regards,
dan carpenter
[Bug 91960] [i915] kernel warning hsw_unclaimed_reg_debug intel_uncore.c:619
https://bugs.freedesktop.org/show_bug.cgi?id=91960 --- Comment #5 from JM9 --- In my case, I'm logging into X without a login manager at tty1 and after a thaw my laptop display does not come up. Not just the external display. This makes hibernate to unusable. https://bugs.freedesktop.org/show_bug.cgi?id=93574 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160326/4fabd699/attachment.html>
[Bug 115251] amdgpu: Black screen + hang with Kaveri
https://bugzilla.kernel.org/show_bug.cgi?id=115251 --- Comment #2 from Bernd Steinhauser --- Thanks, that works. -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 94445] Tonga llvm assert since RegisterCoalescer: Need to check DstReg+SrcReg for missing undef flags
https://bugs.freedesktop.org/show_bug.cgi?id=94445 --- Comment #6 from Vedran MiletiÄ --- (In reply to Nicolai Hähnle from comment #5) > Created attachment 122385 [details] > Failing shader > > The shader still fails to compile. I've contacted Matthias about this. Any news? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160326/4963011b/attachment.html>
[Bug 68571] GPU lockup on AMD Radeon HD6850 with DPM=1
https://bugzilla.kernel.org/show_bug.cgi?id=68571 --- Comment #77 from kilobug at kilobug.org --- For information I just changed my GPU to a brand new R9 380X (which uses the amdgpu driver), so I won't be able to easily do more tests on this bug - I still have the 6850 in a box, so if you're pretty sure it's fixed I can swap the card and swap it back, but it's a "delicate" operation so I won't be able to do it frequently. -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH 04/23] drm: omapdrm: add DSI mapping
Hi Sebastian, Thank you for the patch. On Tuesday 08 Mar 2016 17:39:36 Sebastian Reichel wrote: > This sets proper connector type for DSI connected panels. > > Signed-off-By: Sebastian Reichel Reviewed-by: Laurent Pinchart > --- > drivers/gpu/drm/omapdrm/omap_drv.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c > b/drivers/gpu/drm/omapdrm/omap_drv.c index dfafdb602ad2..a3ff35f5f6cd > 100644 > --- a/drivers/gpu/drm/omapdrm/omap_drv.c > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c > @@ -215,6 +215,8 @@ static int get_connector_type(struct omap_dss_device > *dssdev) return DRM_MODE_CONNECTOR_HDMIA; > case OMAP_DISPLAY_TYPE_DVI: > return DRM_MODE_CONNECTOR_DVID; > + case OMAP_DISPLAY_TYPE_DSI: > + return DRM_MODE_CONNECTOR_DSI; > default: > return DRM_MODE_CONNECTOR_Unknown; > } -- Regards, Laurent Pinchart
[PATCH 06/23] drm: omapdrm: wait for pending operations before updating plane
Hi Sabastian, Thank you for the patch. On Tuesday 08 Mar 2016 17:39:38 Sebastian Reichel wrote: > Updating the plane may interrupt ongoing display > updates, so wait for any pending operations. There's already an omap_atomic_wait_for_completion() call a couple of lines below, do we need two of them ? Why can display update be ongoing there, given that the previous omap_atomic_complete() call did wait for completion before returning ? > Signed-off-By: Sebastian Reichel > --- > drivers/gpu/drm/omapdrm/omap_drv.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c > b/drivers/gpu/drm/omapdrm/omap_drv.c index a3ff35f5f6cd..e142a4245766 > 100644 > --- a/drivers/gpu/drm/omapdrm/omap_drv.c > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c > @@ -95,6 +95,10 @@ static void omap_atomic_complete(struct > omap_atomic_state_commit *commit) /* Apply the atomic update. */ > dispc_runtime_get(); > > + dev_dbg(dev->dev, "omap_atomic_complete"); > + > + omap_atomic_wait_for_completion(dev, old_state); > + > drm_atomic_helper_commit_modeset_disables(dev, old_state); > drm_atomic_helper_commit_planes(dev, old_state, false); > drm_atomic_helper_commit_modeset_enables(dev, old_state); -- Regards, Laurent Pinchart
[Bug 115291] New: system freeze on loading radeon module
https://bugzilla.kernel.org/show_bug.cgi?id=115291 Bug ID: 115291 Summary: system freeze on loading radeon module Product: Drivers Version: 2.5 Kernel Version: 4.5 Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-dri at kernel-bugs.osdl.org Reporter: duemig at posteo.de Regression: No Created attachment 210741 --> https://bugzilla.kernel.org/attachment.cgi?id=210741=edit kernel log captured via netconsole system freezes when radeon module is loaded: 1. screen goes black 2. I can hear the harddrive spin down 3. no reaction on key presses like Ctrl+Alt+Del 4. the graphics card's fans halt immediately and do not start again until the reset button is pressed 5. no kernel panic or error messages in kernel log captured via netconsole $ lspci 01:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI] Curacao PRO [Radeon R7 370 / R9 270/370 OEM] (rev 81) The graphics card works with VESA or fglrx drivers, as well as on windows, so the hardware has to be alright. Steps to freeze the system: 1. start system with radeon module blacklisted 2. stop graphical login manager 3. log into the console and manually load radeon module (# modprobe radeon) I copy my bug report from the Arch Linux bugtracker [1] here, since I encountered it on multiple distributions (Arch Linux, Ubuntu) and on the 4.5 vanilla sources as well. [1] https://bugs.archlinux.org/task/48578 -- You are receiving this mail because: You are watching the assignee of the bug.
[RFC 0/5] drm: Add support for tiny LCD displays
Den 18.03.2016 18:48, skrev Daniel Vetter:
> On Thu, Mar 17, 2016 at 11:00:00PM +0100, Noralf Trønnes wrote:
>> Den 16.03.2016 19:26, skrev Eric Anholt:
>>> Noralf Trønnes writes:
>>>
This is an attempt at providing a DRM version of drivers/staging/fbtft.
I'm sending this early before cleaning up the code hoping to get some
feedback in case there are better ways to structure this.
The tinydrm module provides a very simplified view of DRM for displays that
has onboard video memory and is connected through a slow bus like SPI/I2C.
A driver using tinydrm has to provide a function that will be called from
the dirtyfb ioctl and optionally drm_panel functions which can be used to
set up the display controller and control backlight.
tinydrm also has fbdev support using fb_deferred_io which is tied in
through
the dirtyfb function call. A driver can use the provided deferred work code
to collect dirtyfb calls and schedule display memory updates if it whishes.
The various display controllers can have library modules that handles
display updates.
Display controllers that have a similar register interface as the MIPI
DBI/DCS
controllers can use the lcdreg module for register access.
struct tinydrm_device {
struct drm_device *base;
u32 width, height;
struct drm_panel panel;
[...]
int (*dirtyfb)(struct drm_framebuffer *fb, void *vmem, unsigned flags,
unsigned color, struct drm_clip_rect *clips,
unsigned num_clips);
};
>>> This is awesome!
>>>
>>> I was wondering, have you considered what it would take to DMA
>>> framebuffer contents from somewhere in memory to these displays? Does
>>> that look doable to you?
>> The vast majory of these displays are connected through SPI and the SPI
>> subsystem maps the buffers using the DMA streaming API including support
>> for vmalloc buffers. I have some more details in my reply to Daniel.
>>
>>> I'd love to be able to do something PRIME-like where vc4's doing the
>>> rendering and we're periodically updating the TFT with the result.
>> I think I read somewhere that one drm device could do the rendering and
>> another could scan out the buffer. It would be great if this could be done.
>> Some use these displays on handhold game emulators and the emulator only
>> supports OpenGL or some library needing hw rendering. Currently on the
>> Raspberry Pi this is solved by having a program take snapshots of the gpu
>> framebuffer and copy this into the fbtft fbdev framebuffer.
> Yeah, this is definitely perfect use-case for prime buffer sharing.
> CMA/dma buffer support for tinydrm would be great to make this work
> seamlessly.
I have looked at cma prime and it looks like vaddr is not set.
How do I get the virtual address?
drm_prime_fd_to_handle_ioctl
-> drm_gem_prime_fd_to_handle
-> drm_gem_prime_import
-> drm_gem_cma_prime_import_sg_table:
cma_obj = __drm_gem_cma_create(dev, attach->dmabuf->size);
cma_obj->paddr = sg_dma_address(sgt->sgl);
cma_obj->sgt = sgt;
[PATCH 06/23] drm: omapdrm: wait for pending operations before updating plane
Hi Laurent, On Sat, Mar 26, 2016 at 11:20:00AM +0200, Laurent Pinchart wrote: > On Tuesday 08 Mar 2016 17:39:38 Sebastian Reichel wrote: > > Updating the plane may interrupt ongoing display > > updates, so wait for any pending operations. > > There's already an omap_atomic_wait_for_completion() call a couple of lines > below, do we need two of them ? Why can display update be ongoing there, > given > that the previous omap_atomic_complete() call did wait for completion before > returning? This is a preparation for the manual display update. The planes should not be touched while a manual display update is in progress. I only checked the patches for the N950's manually updated DSI panel, but none of the other supported panel types. Since I wasn't sure what is triggered by the plane update for the other panel types, I kept the second wait_for_completion. I guess I will drop the second wait in the next revision and see what happens. -- Sebastian > > Signed-off-By: Sebastian Reichel > > --- > > drivers/gpu/drm/omapdrm/omap_drv.c | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c > > b/drivers/gpu/drm/omapdrm/omap_drv.c index a3ff35f5f6cd..e142a4245766 > > 100644 > > --- a/drivers/gpu/drm/omapdrm/omap_drv.c > > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c > > @@ -95,6 +95,10 @@ static void omap_atomic_complete(struct > > omap_atomic_state_commit *commit) /* Apply the atomic update. */ > > dispc_runtime_get(); > > > > + dev_dbg(dev->dev, "omap_atomic_complete"); > > + > > + omap_atomic_wait_for_completion(dev, old_state); > > + > > drm_atomic_helper_commit_modeset_disables(dev, old_state); > > drm_atomic_helper_commit_planes(dev, old_state, false); > > drm_atomic_helper_commit_modeset_enables(dev, old_state); > > -- > Regards, > > Laurent Pinchart > -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160326/68639a8a/attachment-0001.sig>
blank screen on boot with i915/DRM_FBDEV_EMULATION
Hi, I just compiled a new kernel version 4.4.5, replacing a previous 4.1.9 for my thinkpad x40, which replaced CONFIG_DRM_I915_FBDEV with CONFIG_DRM_FBDEV_EMULATION--now, when the i915 and fbcon modules get loaded during boot, the screen goes blank (with backlight still on), which didn't happen with the old kernel. The funny thing is that starting an X server makes text reappear on the console!? (And the X server also works just fine, as far as I can tell so far ...) i915 is loaded with modeset=1 (I remember I had some problem without it with some earlier kernels, I don't really remember the details anymore ...). Any hints what to try? Any further information that could be useful? Regards, Florian
[PATCH] drm/edid: Fix parsing of EDID 1.4 Established Timings III descriptor
The EDID 1.4 specification section 3.10.3.9 defines an Established Timings III
descriptor (tag #F7h). The parsing of this descriptor by drm_est3_modes() is
off by one byte: the offset of the first timing bitmap is 6, not 5.
Signed-off-by: Paul Parsons
---
diff -ru a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
--- a/drivers/gpu/drm/drm_edid.c2016-03-14 04:28:54.0 +
+++ b/drivers/gpu/drm/drm_edid.c2016-03-26 12:04:58.963352156 +
@@ -2215,7 +2215,7 @@
{
int i, j, m, modes = 0;
struct drm_display_mode *mode;
- u8 *est = ((u8 *)timing) + 5;
+ u8 *est = ((u8 *)timing) + 6;
for (i = 0; i < 6; i++) {
for (j = 7; j >= 0; j--) {
[PATCH 13/23] drm: omapdrm: crtc: update plane fifos on lcd config change
Hi Sebastian,
Thank you for the patch.
On Tuesday 08 Mar 2016 17:39:45 Sebastian Reichel wrote:
> Due to a hardware bug, FIFOs thresholds must be
> configured very carefully for manually updated
> displays.
Could you please provide more information about the bug and how the code
handles it ?
> Signed-off-by: Sebastian Reichel
> ---
> drivers/gpu/drm/omapdrm/omap_crtc.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 8967013c1fb5..094e89a2fa94
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -253,6 +253,10 @@ static void omap_crtc_dss_set_lcd_config(struct
> omap_overlay_manager *mgr, omap_crtc->manually_updated =
> dss_lcd_mgr_config_get_stallmode(config);
>
> dispc_mgr_set_lcd_config(omap_crtc->channel, config);
> +
> + drm_for_each_plane(plane, dev) {
> + omap_plane_update_fifo(plane);
> + }
This seems fishy :-/ To start with you shouldn't touch planes that don't
belong to this CRTC. Then, updating the FIFO thresholds here in addition to
omap_plane_atomic_update() makes me wonder if both are needed, and if so, why.
> }
>
> static int omap_crtc_dss_register_framedone(
--
Regards,
Laurent Pinchart
[PATCH 14/23] drm: omapdrm: crtc: save framedone callback from dss
Hi Sebastian,
Thank you for the patch.
On Tuesday 08 Mar 2016 17:39:46 Sebastian Reichel wrote:
> Save the framedone callback supplied by dss for later
> usage.
We already have too many callbacks in the driver, making the code difficult to
understand. Wouldn't it be possible to cal directly from the omapdrm driver
into the dss code without using callbacks ?
> Signed-off-by: Sebastian Reichel
> ---
> drivers/gpu/drm/omapdrm/omap_crtc.c | 24
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 094e89a2fa94..3ce7143e5a5f
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -57,6 +57,9 @@ struct omap_crtc {
>
> unsigned long state;
> wait_queue_head_t pending_wait;
> +
> + void (*framedone_handler)(void *);
> + void *framedone_handler_data;
> };
>
> /*
> ---
> -- @@ -263,6 +266,17 @@ static int omap_crtc_dss_register_framedone(
> struct omap_overlay_manager *mgr,
> void (*handler)(void *), void *data)
> {
> + struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
> + 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;
> }
>
> @@ -270,6 +284,16 @@ static void omap_crtc_dss_unregister_framedone(
> struct omap_overlay_manager *mgr,
> void (*handler)(void *), void *data)
> {
> + struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
> + 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 = {
--
Regards,
Laurent Pinchart
[PATCH 11/23] include: video: omapdss: provide fifo threshold methods
Hi Sebastian, Thank you for the patch. On Tuesday 08 Mar 2016 17:39:43 Sebastian Reichel wrote: > The FIFO thresholds must be configured by omapdrm for > manually updated DSI panels due to a hardware bug. I find the subject a bit misleading, it made me think that the patch added new methods, while it only declares them as part of the DSS public API. > Signed-off-By: Sebastian Reichel > --- > include/video/omapdss.h | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/include/video/omapdss.h b/include/video/omapdss.h > index 9bde65b79220..f6cdd809ae5c 100644 > --- a/include/video/omapdss.h > +++ b/include/video/omapdss.h > @@ -935,6 +935,12 @@ int dispc_ovl_setup(enum omap_plane plane, const struct > omap_overlay_info *oi, bool replication, const struct omap_video_timings > *mgr_timings, bool mem_to_mem); > > +void dispc_ovl_compute_fifo_thresholds(enum omap_plane plane, > + u32 *fifo_low, u32 *fifo_high, bool use_fifomerge, > + bool manual_update); > +void dispc_ovl_set_fifo_threshold(enum omap_plane plane, u32 low, u32 > high); Should you then remove the function declarations from drivers/gpu/drm/omapdrm/dss/dss.h ? > + > + One blank line is enough. > int omapdss_compat_init(void); > void omapdss_compat_uninit(void); -- Regards, Laurent Pinchart
[PATCH 10/23] drm: omapdrm: crtc: detect manually updated displays
Hi Sebastian,
Thank you for the patch.
On Tuesday 08 Mar 2016 17:39:42 Sebastian Reichel wrote:
> Signed-off-by: Sebastian Reichel
> ---
> drivers/gpu/drm/omapdrm/omap_crtc.c | 31 ++-
> drivers/gpu/drm/omapdrm/omap_drv.h | 1 +
> 2 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 78ef9773cca1..8967013c1fb5
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -48,6 +48,7 @@ struct omap_crtc {
> struct omap_overlay_manager *mgr;
>
> struct omap_video_timings timings;
> + bool manually_updated;
>
> struct omap_drm_irq vblank_irq;
> struct omap_drm_irq error_irq;
> @@ -90,6 +91,12 @@ int omap_crtc_wait_pending(struct drm_crtc *crtc)
> msecs_to_jiffies(50));
> }
>
> +bool omap_crtc_is_manual_updated(struct drm_crtc *crtc)
> +{
> + struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> + return omap_crtc->manually_updated;
> +}
> +
> /*
> ---
> -- * DSS Manager Functions
> */
> @@ -154,6 +161,11 @@ static void omap_crtc_set_enabled(struct drm_crtc
> *crtc, bool enable) omap_crtc->ignore_digit_sync_lost = true;
> }
>
> + if (omap_crtc->manually_updated) {
> + dev_dbg(dev->dev, "stallmode detected, not waiting for irq");
> + return;
> + }
> +
> framedone_irq = dispc_mgr_get_framedone_irq(channel);
> vsync_irq = dispc_mgr_get_vsync_irq(channel);
>
> @@ -233,7 +245,13 @@ static void omap_crtc_dss_set_lcd_config(struct
> omap_overlay_manager *mgr, const struct dss_lcd_mgr_config *config)
> {
> struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
> - DBG("%s", omap_crtc->name);
> + struct drm_device *dev = omap_crtc->base.dev;
> + struct drm_plane *plane;
> +
> + dev_dbg(dev->dev, "set lcd config for %s", omap_crtc->name);
> +
> + omap_crtc->manually_updated = dss_lcd_mgr_config_get_stallmode(config);
> +
> dispc_mgr_set_lcd_config(omap_crtc->channel, config);
> }
>
> @@ -358,10 +376,18 @@ static bool omap_crtc_mode_fixup(struct drm_crtc
> *crtc, static void omap_crtc_enable(struct drm_crtc *crtc)
> {
> struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> + struct omap_dss_device *display = omap_crtc->mgr->output->dst;
> struct drm_device *dev = crtc->dev;
>
> DBG("%s", omap_crtc->name);
>
> + /* manual updated display will not trigger vsync irq */
> + /* omap_crtc->manually_updated is not yet set */
Can't you figure out whether the display requires manual update earlier,
preferably during atomic_check, and store the value in the CRTC state ?
> + if (display->caps & OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE) {
> + dev_dbg(dev->dev, "manual update display detected!");
> + return;
> + }
> +
> if (test_and_set_bit(crtc_pending, _crtc->state))
> dev_warn(dev->dev, "crtc enable while pending bit set!");
>
> @@ -407,6 +433,9 @@ static void omap_crtc_atomic_flush(struct drm_crtc
> *crtc,
>
> WARN_ON(omap_crtc->vblank_irq.registered);
>
> + if (omap_crtc->manually_updated)
> + return;
> +
> if (dispc_mgr_is_enabled(omap_crtc->channel)) {
>
> DBG("%s: GO", omap_crtc->name);
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h
> b/drivers/gpu/drm/omapdrm/omap_drv.h index 5dfa93a3b505..71e2c2284b86
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.h
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.h
> @@ -155,6 +155,7 @@ void omap_crtc_pre_uninit(void);
> struct drm_crtc *omap_crtc_init(struct drm_device *dev,
> struct drm_plane *plane, enum omap_channel channel, int id);
> int omap_crtc_wait_pending(struct drm_crtc *crtc);
> +bool omap_crtc_is_manual_updated(struct drm_crtc *crtc);
>
> struct drm_plane *omap_plane_init(struct drm_device *dev,
> int id, enum drm_plane_type type);
--
Regards,
Laurent Pinchart
[PATCH 08/23] drm: omapdrm: crtc: add enabled bit to state
Hi Sebastian,
Thank you for the patch.
Given that you only add four lines of code here, and given that the enabled
bit is unused here, I'd just squash this patch with the one that makes use of
the enabled bit.
On Tuesday 08 Mar 2016 17:39:40 Sebastian Reichel wrote:
> Signed-off-by: Sebastian Reichel
> ---
> drivers/gpu/drm/omapdrm/omap_crtc.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 5ef27664bcfa..78ef9773cca1
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -207,6 +207,8 @@ static int omap_crtc_dss_enable(struct
> omap_overlay_manager *mgr) _crtc->timings);
> omap_crtc_set_enabled(_crtc->base, true);
>
> + set_bit(crtc_enabled, _crtc->state);
> +
Atomic KMS drivers should store state in the state structure, not the KMS
objects themselves. Beside, how does this differ from the CRTC state enable
and active fields ?
> return 0;
> }
>
> @@ -214,6 +216,8 @@ static void omap_crtc_dss_disable(struct
> omap_overlay_manager *mgr) {
> struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
>
> + clear_bit(crtc_enabled, _crtc->state);
> +
> omap_crtc_set_enabled(_crtc->base, false);
> }
--
Regards,
Laurent Pinchart
[PATCH 07/23] drm: omapdrm: crtc: switch pending variable to atomic bitset
Hi Sebastian,
Thank you for the patch.
On Tuesday 08 Mar 2016 17:39:39 Sebastian Reichel wrote:
> Having the pending variable available as atomic bit helps
> with the later addition of manually updated display support.
>
> Signed-off-by: Sebastian Reichel
> ---
> drivers/gpu/drm/omapdrm/omap_crtc.c | 31 +--
> 1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 2ed0754ed19e..5ef27664bcfa
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -28,6 +28,11 @@
>
> #define to_omap_crtc(x) container_of(x, struct omap_crtc, base)
>
> +enum omap_crtc_state {
I find that using an enum and calling it omap_crtc_state is confusing, it
seems to imply that the CRTC state will be one of the enumerated values, while
the values are actually non-exclusive bits in a bitmask.
> + crtc_enabled= 0,
You don't seem to be using this bit in this patch, you can define it in patch
08/23.
> + crtc_pending= 1
Please name this OMAP_CRTC_STATE_PENDING.
> +};
Please add a short description of each bit in a comment above the enum.
> +
> struct omap_crtc {
> struct drm_crtc base;
>
> @@ -49,7 +54,7 @@ struct omap_crtc {
>
> bool ignore_digit_sync_lost;
>
> - bool pending;
> + unsigned long state;
> wait_queue_head_t pending_wait;
> };
>
> @@ -81,7 +86,7 @@ int omap_crtc_wait_pending(struct drm_crtc *crtc)
> struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
>
> return wait_event_timeout(omap_crtc->pending_wait,
> - !omap_crtc->pending,
> + !test_bit(crtc_pending, _crtc->state),
> msecs_to_jiffies(50));
> }
>
> @@ -311,10 +316,8 @@ static void omap_crtc_vblank_irq(struct omap_drm_irq
> *irq, uint32_t irqstatus)
>
> __omap_irq_unregister(dev, _crtc->vblank_irq);
>
> - rmb();
> - WARN_ON(!omap_crtc->pending);
> - omap_crtc->pending = false;
> - wmb();
> + if (!test_and_clear_bit(crtc_pending, _crtc->state))
> + dev_warn(dev->dev, "pending bit was not set in vblank irq");
Documentation/atomic_ops.txt states that the atomic bit ops must be atomic and
include memory barriers, but I'm confused by the ARM implementation.
The constant bit number version atomic_test_and_clear_bit() uses
raw_local_irq_save() and raw_low_irq_restore() for synchronization, which
expand to arch_local_irq_save() and arch_local_irq_restore(). On ARMv6 and
newer, the functions are defined as
static inline unsigned long arch_local_irq_save(void)
{
unsigned long flags;
asm volatile(
" mrs %0, " IRQMASK_REG_NAME_R " @
arch_local_irq_save\n"
" cpsid i"
: "=r" (flags) : : "memory", "cc");
return flags;
}
and
static inline void arch_local_irq_restore(unsigned long flags)
{
asm volatile(
" msr " IRQMASK_REG_NAME_W ", %0 @
local_irq_restore"
:
: "r" (flags)
: "memory", "cc");
}
I'm probably missing something obvious, but I don't see the memory barriers
:-/
> /* wake up userspace */
> omap_crtc_complete_page_flip(_crtc->base);
> @@ -351,13 +354,12 @@ static bool omap_crtc_mode_fixup(struct drm_crtc
> *crtc, static void omap_crtc_enable(struct drm_crtc *crtc)
> {
> struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> + struct drm_device *dev = crtc->dev;
>
> DBG("%s", omap_crtc->name);
>
> - rmb();
> - WARN_ON(omap_crtc->pending);
> - omap_crtc->pending = true;
> - wmb();
> + if (test_and_set_bit(crtc_pending, _crtc->state))
> + dev_warn(dev->dev, "crtc enable while pending bit set!");
>
> omap_irq_register(crtc->dev, _crtc->vblank_irq);
>
> @@ -397,6 +399,7 @@ static void omap_crtc_atomic_flush(struct drm_crtc
> *crtc, struct drm_crtc_state *old_crtc_state) {
> struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> + struct drm_device *dev = crtc->dev;
>
> WARN_ON(omap_crtc->vblank_irq.registered);
>
> @@ -404,10 +407,8 @@ static void omap_crtc_atomic_flush(struct drm_crtc
> *crtc,
>
> DBG("%s: GO", omap_crtc->name);
>
> - rmb();
> - WARN_ON(omap_crtc->pending);
> - omap_crtc->pending = true;
> - wmb();
> + if (test_and_set_bit(crtc_pending, _crtc->state))
> + dev_warn(dev->dev, "atomic flush while pending bit
> set!");
>
> dispc_mgr_go(omap_crtc->channel);
> omap_irq_register(crtc->dev, _crtc->vblank_irq);
> @@ -509,6 +510,8 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
>
> init_waitqueue_head(_crtc->pending_wait);
>
> + omap_crtc->state = 0;
> +
> omap_crtc->channel
