drm/ticdc: Accumulated fixes over the past couple of years
Subsystems (3)
v4 cover drm/ticdc: Accumulated fixes over the past couple of years
... and yet one more round.
Changes since v3:
- 21/22 "drm/tilcdc: Initialize crtc->port"
- Return an error and don't use scary WARN but simple dev_err() instead
- 22/22 "drm/tilcdc: Use devm_kzalloc() and devm_kcalloc() for private data"
- Tested module loading and unloading with all relevant (and prbably some unrelevant) memory debugging option on. No problem found in the patch. However, there is something fishy either in the component framework or in the way tilcdc used, but that is another story.
One more round to fix the last issues commented by Tomi Valkeinen. Changes since v2:
- 09/22 "drm/tilcdc: Allocate register storage based on the actual number registers"
- Fixed typo in the description
- 21/22 "drm/tilcdc: Initialize crtc->port"
- Changed the code to handle a port-node that is put in side a ports-node
- Removed dev_warn(), and use WARN_ON() instead
- Added more details in description
- 22/22 "Use devm_kzalloc() and devm_kcalloc() for private data"
- New patch
Changes since the first version of the series:
- Dropped: "drm/tilcdc: disable console switching during pm operations"
- Changed: "drm/tilcdc: Allocate register storage based on the actual number.."
- Reversed kcalloc() nmemb and size parameters to correct order
- Added: "drm/tilcdc: Initialize crtc->port"
We have not been too active in pushing the tilcdc fixes to mainline. This series tries to bring the mainline tilcdc upto same level with TI ti-linux tree.
Some patches that touch the same place over and over again have been squashed into one, leaving author of the last rewrite on top.
Best regards,
Jyri
Darren Etheridge (4):
drm/tilcdc: rewrite pixel clock calculation
drm/tilcdc: fix kernel panic on suspend when no hdmi monitor connected drm/tilcdc: make frame_done interrupt active at all times drm/tilcdc: disable the lcd controller/dma engine when suspend invoked
Dave Gerlach (1):
drm/tilcdc: adopt pinctrl support
Grygorii Strashko (1):
drm/tilcdc: fix build error when !CONFIG_CPU_FREQ
Jyri Sarha (8):
drm/tilcdc: Implement dma-buf support for tilcdc drm/tilcdc: Allocate register storage based on the actual number
registers
drm/tilcdc: Fix interrupt enable/disable code for version 2 tilcdc drm/tilcdc: Remove the duplicate LCDC_INT_ENABLE_SET_REG in
registers[]
drm/tilcdc: Add prints on sync lost and FIFO underrun interrupts drm/tilcdc: Disable sync lost interrupt if it fires on every frame drm/tilcdc: Initialize crtc->port
drm/tilcdc: Use devm_kzalloc() and devm_kcalloc() for private data
Tomi Valkeinen (8):
drm/tilcdc: verify fb pitch
drm/tilcdc: cleanup runtime PM handling
drm/tilcdc: disable crtc on unload
drm/tilcdc: split reset to a separate function drm/tilcdc: remove broken error handling
drm/tilcdc: cleanup irq handling
drm/tilcdc: Get rid of complex ping-pong mechanism drm/tilcdc: Do not update the next frame buffer close to vertical
blank
drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 313 ++++++++++++++++++++++----------- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 130 ++++++++------ drivers/gpu/drm/tilcdc/tilcdc_drv.h | 5 +- drivers/gpu/drm/tilcdc/tilcdc_panel.c | 20 +-- drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 24 +-- 5 files changed, 300 insertions(+), 192 deletions(-)
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 1/22 drm/tilcdc: rewrite pixel clock calculation
From: Darren Etheridge <[email protected]>
Updating the tilcdc DRM driver code to calculate the LCD controller pixel clock more accurately. Based on a suggested implementation by Tomi Valkeinen.
The current code does not work correctly and produces wrong results with many requested clock rates. It also oddly uses two different clocks, a display pll clock and a divider clock (child of display pll), instead of just using the clock coming to the lcdc.
This patch removes the use of the display pll clock, and rewrites the code to calculate the clock rates. The idea is simply to request a clock rate of pixelclock*2, as the LCD controller has an internal divider which we set to 2.
--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 16 ++++++++-------- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 11 +---------- drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 - 3 files changed, 9 insertions(+), 19 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +8 -8
@@ -573,7 +573,8 @@ void tilcdc_crtc_update_clk(struct drm_crtc *crtc)struct drm_device *dev = crtc->dev;struct tilcdc_drm_private *priv = dev->dev_private;int dpms = tilcdc_crtc->dpms; -unsigned int lcd_clk, div; +unsigned long lcd_clk; +const unsigned clkdiv =2 ;/* using a fixed divider of 2 */ int ret;pm_runtime_get_sync (dev->dev);
@@ -581,22 +582,21 @@ void tilcdc_crtc_update_clk(struct drm_crtc *crtc)if (dpms ==DRM_MODE_DPMS_ON )tilcdc_crtc_dpms (crtc,DRM_MODE_DPMS_OFF ); -/* in raster mode, minimum divisor is 2: */ - ret =clk_set_rate (priv->disp_clk, crtc->mode.clock *1000 *2 ); -if (ret) { +/* mode.clock is in KHz, set_rate wants parameter in Hz */ + ret =clk_set_rate (priv->clk, crtc->mode.clock *1000 * clkdiv); +if (ret <0 ) {dev_err (dev->dev,"failed to set display clock rate to: %d\n" , crtc->mode.clock); goto out; } lcd_clk =clk_get_rate (priv->clk); - div = lcd_clk / (crtc->mode.clock *1000 ); -DBG ("lcd_clk=%u, mode clock=%d, div=%u" , lcd_clk, crtc->mode.clock, div); -DBG ("fck=%lu, dpll_disp_ck=%lu" ,clk_get_rate (priv->clk),clk_get_rate (priv->disp_clk)); +DBG ("lcd_clk=%lu, mode clock=%d, div=%u" , + lcd_clk, crtc->mode.clock, clkdiv);/* Configure the LCD clock divisor. */ -tilcdc_write (dev,LCDC_CTRL_REG ,LCDC_CLK_DIVISOR (div) | +tilcdc_write (dev,LCDC_CTRL_REG ,LCDC_CLK_DIVISOR (clkdiv) |LCDC_RASTER_MODE );if (priv->rev ==2 )
drivers/gpu/drm/tilcdc/tilcdc_drv.c +1 -10
@@ -192,13 +192,6 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags) goto fail_iounmap; } - priv->disp_clk =clk_get (dev->dev,"dpll_disp_ck" ); -if (IS_ERR (priv->clk)) { -dev_err (dev->dev,"failed to get display clock\n" ); - ret = -ENODEV ; - goto fail_put_clk; - } -#ifdef CONFIG_CPU_FREQ priv->lcd_fck_rate =clk_get_rate (priv->clk); priv->freq_transition.notifier_call = cpufreq_transition;
@@ -206,7 +199,7 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags) CPUFREQ_TRANSITION_NOTIFIER);if (ret) {dev_err (dev->dev,"failed to register cpufreq notifier\n" ); - goto fail_put_disp_clk; + goto fail_put_clk; }#endif
@@ -330,8 +323,6 @@ fail_cpufreq_unregister:#ifdef CONFIG_CPU_FREQ cpufreq_unregister_notifier (&priv->freq_transition,CPUFREQ_TRANSITION_NOTIFIER ); -fail_put_disp_clk: -clk_put (priv->disp_clk);#endif fail_put_clk:
drivers/gpu/drm/tilcdc/tilcdc_drv.h +0 -1
@@ -49,7 +49,6 @@struct tilcdc_drm_private {void __iomem *mmio; -struct clk *disp_clk;/* display dpll */ struct clk *clk;/* functional clock */ int rev;/* IP revision */
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 2/22 drm/tilcdc: verify fb pitch
In reply to Jyri SarhaFrom: Tomi Valkeinen <[email protected]>
LCDC hardware does not support fb pitch that is different (i.e. larger) than the screen size. The driver currently does no checks for this, and the results of too big pitch are are flickering and lower fps.
This issue easily happens when using libdrm's modetest tool with non-32 bpp modes. As modetest always allocated 4 bytes per pixel, it implies a bigger pitch for 16 or 24 bpp modes.
This patch adds a check to reject pitches the hardware cannot support.
--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +31 -0
@@ -151,6 +151,22 @@ static void tilcdc_crtc_destroy(struct drm_crtc *crtc)kfree (tilcdc_crtc); } +static int tilcdc_verify_fb (struct drm_crtc *crtc,struct drm_framebuffer *fb) +{ +struct drm_device *dev = crtc->dev; +unsigned int depth, bpp; + +drm_fb_get_bpp_depth (fb->pixel_format, &depth, &bpp); + +if (fb->pitches[0 ] != crtc->mode.hdisplay * bpp /8 ) { +dev_err (dev->dev, +"Invalid pitch: fb and crtc widths must be the same" ); +return -EINVAL ; + } + +return 0 ; +} + static int tilcdc_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, struct drm_pending_vblank_event *event,
@@ -158,6 +174,11 @@ static int tilcdc_crtc_page_flip(struct drm_crtc *crtc, {struct tilcdc_crtc *tilcdc_crtc =to_tilcdc_crtc (crtc);struct drm_device *dev = crtc->dev; +int r; + + r =tilcdc_verify_fb (crtc, fb); +if (r) +return r;if (tilcdc_crtc->event) {dev_err (dev->dev,"already pending page flip!\n" );
@@ -272,6 +293,10 @@ static int tilcdc_crtc_mode_set(struct drm_crtc *crtc,if (WARN_ON (!info))return -EINVAL ; + ret =tilcdc_verify_fb (crtc, crtc->primary->fb); +if (ret) +return ret; +pm_runtime_get_sync (dev->dev);/* Configure the Burst Size and fifo threshold of DMA: */
@@ -431,6 +456,12 @@ static int tilcdc_crtc_mode_set(struct drm_crtc *crtc,static int tilcdc_crtc_mode_set_base (struct drm_crtc *crtc,int x,int y,struct drm_framebuffer *old_fb) { +int r; + + r =tilcdc_verify_fb (crtc, crtc->primary->fb); +if (r) +return r; +update_scanout (crtc);return 0 ; }
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 3/22 drm/tilcdc: adopt pinctrl support
In reply to Jyri SarhaFrom: Dave Gerlach <[email protected]>
Update tilcdc driver to set the state of the pins to:
- "default on resume
- "sleep" on suspend
By optionally putting the pins into sleep state in the suspend callback we can accomplish two things.
- minimize current leakage from pins and thus save power,
- prevent the IP from driving pins output in an uncontrolled manner, which may happen if the power domain drops the domain regulator.
--- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/gpu/drm/tilcdc/tilcdc_drv.c +8 -0
@@ -18,6 +18,8 @@/* LCDC DRM driver, based on da8xx-fb */ #include <linux/component.h> +#include <linux/pinctrl/consumer.h> +#include <linux/suspend.h> #include "tilcdc_drv.h" #include "tilcdc_regs.h"
@@ -585,6 +587,9 @@ static int tilcdc_pm_suspend(struct device *dev)if (registers[i].save && (priv->rev >= registers[i].rev)) priv->saved_register[n++] =tilcdc_read (ddev, registers[i].reg); +/* Select sleep pin state */ +pinctrl_pm_select_sleep_state (dev); +return 0 ; }
@@ -594,6 +599,9 @@ static int tilcdc_pm_resume(struct device *dev)struct tilcdc_drm_private *priv = ddev->dev_private;unsigned i, n =0 ; +/* Select default pin state */ +pinctrl_pm_select_default_state (dev); +/* Restore register state: */ for (i =0 ; i <ARRAY_SIZE (registers); i++)if (registers[i].save && (priv->rev >= registers[i].rev))
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 4/22 drm/tilcdc: fix kernel panic on suspend when no hdmi monitor connected
In reply to Jyri SarhaFrom: Darren Etheridge <[email protected]>
On BeagleBone Black if no HDMI monitor is connected and suspend is requested a kernel panic will result:
root@am335x-evm:~# echo mem > /sys/power/state [ 65.548710] PM: Syncing filesystems ... done. [ 65.631311] Freezing user space processes ... (elapsed 0.006 seconds) done. [ 65.648619] Freezing remaining freezable tasks ... (elapsed 0.005 seconds) done. [ 65.833500] Unhandled fault: external abort on non-linefetch (0x1028) at 0xfa30e004 [ 65.841692] Internal error: : 1028 [#1] SMP ARM <snip> [ 66.105287] [<c03765f0>] (platform_pm_suspend) from [<c037b6d4>] (dpm_run_callback+0x34/0x70) [ 66.114370] [<c037b6d4>] (dpm_run_callback) from [<c037ba84>] (__device_suspend+0x10c/0x2f4) [ 66.123357] [<c037ba84>] (__device_suspend) from [<c037d004>] (dpm_suspend+0x58/0x218) [ 66.131796] [<c037d004>] (dpm_suspend) from [<c008d948>] (suspend_devices_and_enter+0x9c/0x3c0) [ 66.141055] [<c008d948>] (suspend_devices_and_enter) from [<c008de7c>] (pm_suspend+0x210/0x24c) [ 66.150312] [<c008de7c>] (pm_suspend) from [<c008cabc>] (state_store+0x68/0xb8) [ 66.158103] [<c008cabc>] (state_store) from [<c02e9654>] (kobj_attr_store+0x14/0x20) [ 66.166355] [<c02e9654>] (kobj_attr_store) from [<c0185c70>] (sysfs_kf_write+0x4c/0x50) [ 66.174883] [<c0185c70>] (sysfs_kf_write) from [<c018926c>] (kernfs_fop_write+0xb4/0x150) [ 66.183598] [<c018926c>] (kernfs_fop_write) from [<c0122638>] (vfs_write+0xa8/0x180) [ 66.191846] [<c0122638>] (vfs_write) from [<c01229f8>] (SyS_write+0x40/0x8c) [ 66.199365] [<c01229f8>] (SyS_write) from [<c000e580>] (ret_fast_syscall+0x0/0x48) [ 66.207426] Code: e595c210 e5932000 e59cc000 e08c2002 (e592c000)
This is because the lcdc module is not enabled when no monitor is detected to save power. However the suspend handler just blindly tries to save the lcdc state by copying out the pertinent registers. However module is off so no good things happen when you try and access it.
This patch only saves off the registers if the module is enabled, and then only restores the registers on resume if they were saved off during suspend.
--- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 23 +++++++++++++++++------ drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 + 2 files changed, 18 insertions(+), 6 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_drv.c +17 -6
@@ -582,13 +582,20 @@ static int tilcdc_pm_suspend(struct device *dev)drm_kms_helper_poll_disable (ddev); +/* Select sleep pin state */ +pinctrl_pm_select_sleep_state (dev); + +if (pm_runtime_suspended (dev)) { + priv->ctx_valid = false; +return 0 ; + } +/* Save register state: */ for (i =0 ; i <ARRAY_SIZE (registers); i++)if (registers[i].save && (priv->rev >= registers[i].rev)) priv->saved_register[n++] =tilcdc_read (ddev, registers[i].reg); -/* Select sleep pin state */ -pinctrl_pm_select_sleep_state (dev); + priv->ctx_valid = true;return 0 ; }
@@ -602,10 +609,14 @@ static int tilcdc_pm_resume(struct device *dev)/* Select default pin state */ pinctrl_pm_select_default_state (dev); -/* Restore register state: */ -for (i =0 ; i <ARRAY_SIZE (registers); i++) -if (registers[i].save && (priv->rev >= registers[i].rev)) -tilcdc_write (ddev, registers[i].reg, priv->saved_register[n++]); +if (priv->ctx_valid == true) { +/* Restore register state: */ +for (i =0 ; i <ARRAY_SIZE (registers); i++) +if (registers[i].save && + (priv->rev >= registers[i].rev)) +tilcdc_write (ddev, registers[i].reg, + priv->saved_register[n++]); + }drm_kms_helper_poll_enable (ddev);
drivers/gpu/drm/tilcdc/tilcdc_drv.h +1 -0
@@ -67,6 +67,7 @@ struct tilcdc_drm_private {/* register contents saved across suspend/resume: */ u32 saved_register[12 ]; +bool ctx_valid;#ifdef CONFIG_CPU_FREQ struct notifier_block freq_transition;
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 5/22 drm/tilcdc: make frame_done interrupt active at all times
In reply to Jyri SarhaFrom: Darren Etheridge <[email protected]>
The frame_done interrupt was only being enabled when the vsync interrupts were being enabled by DRM. However the frame_done is used to determine if the LCD controller has successfully completed the raster_enable, raster_disable commands and the vsync interrupts are not always enabled during these operations.
--- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_drv.c +4 -2
@@ -369,7 +369,9 @@ static int tilcdc_irq_postinstall(struct drm_device *dev)if (priv->rev ==1 )tilcdc_set (dev,LCDC_RASTER_CTRL_REG ,LCDC_V1_UNDERFLOW_INT_ENA );else -tilcdc_set (dev,LCDC_INT_ENABLE_SET_REG ,LCDC_V2_UNDERFLOW_INT_ENA ); +tilcdc_set (dev,LCDC_INT_ENABLE_SET_REG , +LCDC_V2_UNDERFLOW_INT_ENA | +LCDC_FRAME_DONE );return 0 ; }
@@ -403,7 +405,7 @@ static void enable_vblank(struct drm_device *dev, bool enable) }else { reg =LCDC_INT_ENABLE_SET_REG ; mask =LCDC_V2_END_OF_FRAME0_INT_ENA | -LCDC_V2_END_OF_FRAME1_INT_ENA |LCDC_FRAME_DONE ; +LCDC_V2_END_OF_FRAME1_INT_ENA ; }if (enable)
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 6/22 drm/tilcdc: disable the lcd controller/dma engine when suspend invoked
In reply to Jyri SarhaFrom: Darren Etheridge <[email protected]>
The LCD controller must be deactivated and all DMA transactions stopped when the suspend power state is entered otherwise the PRCM causes the L3 bus to get stuck in transition state.
This commit forces the lcdc to be shut down and waits for all pending DMA transactions to complete as part of the suspend handler for this driver.
--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 3 +-- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 3 +++ drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 + 3 files changed, 5 insertions(+), 2 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +1 -2
@@ -138,7 +138,6 @@ static void stop(struct drm_crtc *crtc)tilcdc_clear (dev,LCDC_RASTER_CTRL_REG ,LCDC_RASTER_ENABLE ); } -static void tilcdc_crtc_dpms (struct drm_crtc *crtc,int mode);static void tilcdc_crtc_destroy (struct drm_crtc *crtc) {struct tilcdc_crtc *tilcdc_crtc =to_tilcdc_crtc (crtc);
@@ -192,7 +191,7 @@ static int tilcdc_crtc_page_flip(struct drm_crtc *crtc,return 0 ; } -static void tilcdc_crtc_dpms (struct drm_crtc *crtc,int mode) +void tilcdc_crtc_dpms (struct drm_crtc *crtc,int mode) {struct tilcdc_crtc *tilcdc_crtc =to_tilcdc_crtc (crtc);struct drm_device *dev = crtc->dev;
drivers/gpu/drm/tilcdc/tilcdc_drv.c +3 -0
@@ -592,6 +592,9 @@ static int tilcdc_pm_suspend(struct device *dev)return 0 ; } +/* Disable the LCDC controller, to avoid locking up the PRCM */ +tilcdc_crtc_dpms (priv->crtc,DRM_MODE_DPMS_OFF ); +/* Save register state: */ for (i =0 ; i <ARRAY_SIZE (registers); i++)if (registers[i].save && (priv->rev >= registers[i].rev))
drivers/gpu/drm/tilcdc/tilcdc_drv.h +1 -0
@@ -171,5 +171,6 @@ void tilcdc_crtc_set_simulate_vesa_sync(struct drm_crtc *crtc, bool simulate_vesa_sync);int tilcdc_crtc_mode_valid (struct drm_crtc *crtc,struct drm_display_mode *mode);int tilcdc_crtc_max_width (struct drm_crtc *crtc); +void tilcdc_crtc_dpms (struct drm_crtc *crtc,int mode);#endif /* __TILCDC_DRV_H__ */
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 7/22 drm/tilcdc: Implement dma-buf support for tilcdc
In reply to Jyri SarhaThere is nothing special about tilcdc HW when the video memory is concerned. Just using the standard drm helpers for implementation is enough.
--- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
drivers/gpu/drm/tilcdc/tilcdc_drv.c +12 -1
@@ -542,7 +542,8 @@ static const struct file_operations fops = { };static struct drm_driver tilcdc_driver = { - .driver_features =DRIVER_HAVE_IRQ |DRIVER_GEM |DRIVER_MODESET , + .driver_features = (DRIVER_HAVE_IRQ |DRIVER_GEM |DRIVER_MODESET | +DRIVER_PRIME ), .load = tilcdc_load, .unload = tilcdc_unload, .lastclose = tilcdc_lastclose,
@@ -559,6 +560,16 @@ static struct drm_driver tilcdc_driver = { .dumb_create = drm_gem_cma_dumb_create, .dumb_map_offset = drm_gem_cma_dumb_map_offset, .dumb_destroy = drm_gem_dumb_destroy, + + .prime_handle_to_fd = drm_gem_prime_handle_to_fd, + .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .gem_prime_import = drm_gem_prime_import, + .gem_prime_export = drm_gem_prime_export, + .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, + .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, + .gem_prime_vmap = drm_gem_cma_prime_vmap, + .gem_prime_vunmap = drm_gem_cma_prime_vunmap, + .gem_prime_mmap = drm_gem_cma_prime_mmap, #ifdef CONFIG_DEBUG_FS .debugfs_init = tilcdc_debugfs_init, .debugfs_cleanup = tilcdc_debugfs_cleanup,
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 9/22 drm/tilcdc: Allocate register storage based on the actual number registers
In reply to Jyri SarhaAllocate suspend/resume register storage based on the actual number registers the driver is aware of. The static allocation for register storage had fallen behind badly.
--- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 21 ++++++++++++++++++++- drivers/gpu/drm/tilcdc/tilcdc_drv.h | 2 +- 2 files changed, 21 insertions(+), 2 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_drv.c +20 -1
@@ -141,11 +141,14 @@ static int tilcdc_unload(struct drm_device *dev)pm_runtime_disable (dev->dev); +kfree (priv->saved_register);kfree (priv);return 0 ; } +static size_t tilcdc_num_regs (void ); +static int tilcdc_load (struct drm_device *dev,unsigned long flags) {struct platform_device *pdev = dev->platformdev;
@@ -157,7 +160,12 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)int ret; priv =kzalloc (sizeof (*priv),GFP_KERNEL ); -if (!priv) { +if (priv) + priv->saved_register =kcalloc (tilcdc_num_regs (), +sizeof (*priv->saved_register), +GFP_KERNEL ); +if (!priv || !priv->saved_register) { +kfree (priv);dev_err (dev->dev,"failed to allocate private data\n" );return -ENOMEM ; }
@@ -339,6 +347,7 @@ fail_free_wq: fail_free_priv: dev->dev_private =NULL ; +kfree (priv->saved_register);kfree (priv);return ret; }
@@ -456,6 +465,16 @@ static const struct { REG(2, true, LCDC_INT_ENABLE_SET_REG),#undef REG }; + +static size_t tilcdc_num_regs (void ) +{ +return ARRAY_SIZE (registers); +} +#else +static size_t tilcdc_num_regs (void ) +{ +return 0 ; +}#endif #ifdef CONFIG_DEBUG_FS
drivers/gpu/drm/tilcdc/tilcdc_drv.h +1 -1
@@ -66,7 +66,7 @@ struct tilcdc_drm_private {uint32_t max_width;/* register contents saved across suspend/resume: */ -u32 saved_register[12 ]; +u32 *saved_register;bool ctx_valid;#ifdef CONFIG_CPU_FREQ
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 8/22 drm/tilcdc: fix build error when !CONFIG_CPU_FREQ
In reply to Jyri SarhaFrom: Grygorii Strashko <[email protected]>
Fix build error when !CONFIG_CPU_FREQdrivers/gpu/drm/tilcdc/tilcdc_drv.c: In function 'tilcdc_load':drivers/gpu/drm/tilcdc/tilcdc_drv.c:327:1: error: label 'fail_put_clk' defined but not used [-Werror=unused-label]
fail_put_clk:
^
--- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/tilcdc/tilcdc_drv.c +1 -1
@@ -325,9 +325,9 @@ fail_cpufreq_unregister:#ifdef CONFIG_CPU_FREQ cpufreq_unregister_notifier (&priv->freq_transition,CPUFREQ_TRANSITION_NOTIFIER ); -#endif fail_put_clk: +#endif clk_put (priv->clk); fail_iounmap:
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 10/22 drm/tilcdc: cleanup runtime PM handling
In reply to Jyri SarhaFrom: Tomi Valkeinen <[email protected]>
Cleanup runtime PM handling. Before the patch the usage of pm_runtime calls was inconsistent and hard to follow. After the update the pm_runtime calls are removed from set_scanout() and called around major operations that access the HW. After the patch the DPMS code does not have pm_runtime_forbid/allow calls any more and pm_runtime_irq_safe() is not set anymore.
--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 19 +++++++++++-------- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 1 - 2 files changed, 11 insertions(+), 9 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +11 -8
@@ -71,7 +71,6 @@ static void set_scanout(struct drm_crtc *crtc, int n)struct drm_device *dev = crtc->dev;struct tilcdc_drm_private *priv = dev->dev_private; -pm_runtime_get_sync (dev->dev);tilcdc_write (dev, base_reg[n], tilcdc_crtc->start);tilcdc_write (dev, ceil_reg[n], tilcdc_crtc->end);if (tilcdc_crtc->scanout[n]) {
@@ -81,7 +80,6 @@ static void set_scanout(struct drm_crtc *crtc, int n) tilcdc_crtc->scanout[n] = crtc->primary->fb;drm_framebuffer_reference (tilcdc_crtc->scanout[n]); tilcdc_crtc->dirty &= ~stat[n]; -pm_runtime_put_sync (dev->dev); }static void update_scanout (struct drm_crtc *crtc)
@@ -186,8 +184,13 @@ static int tilcdc_crtc_page_flip(struct drm_crtc *crtc, crtc->primary->fb = fb; tilcdc_crtc->event = event; + +pm_runtime_get_sync (dev->dev); +update_scanout (crtc); +pm_runtime_put_sync (dev->dev); +return 0 ; }
@@ -206,10 +209,8 @@ void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode) tilcdc_crtc->dpms = mode; -pm_runtime_get_sync (dev->dev); -if (mode ==DRM_MODE_DPMS_ON ) { -pm_runtime_forbid (dev->dev); +pm_runtime_get_sync (dev->dev);start (crtc); }else { tilcdc_crtc->frame_done = false;
@@ -227,10 +228,9 @@ void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode)if (ret ==0 )dev_err (dev->dev,"timeout waiting for framedone\n" ); } -pm_runtime_allow (dev->dev); - } -pm_runtime_put_sync (dev->dev); +pm_runtime_put_sync (dev->dev); + } } static bool tilcdc_crtc_mode_fixup(struct drm_crtc *crtc,
@@ -455,13 +455,16 @@ static int tilcdc_crtc_mode_set(struct drm_crtc *crtc,static int tilcdc_crtc_mode_set_base (struct drm_crtc *crtc,int x,int y,struct drm_framebuffer *old_fb) { +struct drm_device *dev = crtc->dev;int r; r =tilcdc_verify_fb (crtc, crtc->primary->fb);if (r)return r; +pm_runtime_get_sync (dev->dev);update_scanout (crtc); +pm_runtime_put_sync (dev->dev);return 0 ; }
drivers/gpu/drm/tilcdc/tilcdc_drv.c +0 -1
@@ -230,7 +230,6 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)DBG ("Maximum Pixel Clock Value %dKHz" , priv->max_pixelclock);pm_runtime_enable (dev->dev); -pm_runtime_irq_safe (dev->dev);/* Determine LCD IP Version */ pm_runtime_get_sync (dev->dev);
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 11/22 drm/tilcdc: disable crtc on unload
In reply to Jyri SarhaFrom: Tomi Valkeinen <[email protected]>
Disable crtc on unload. Call tilcdc_crtc_dpms() with DRM_MODE_DPMS_OFF in the beginning of unload function.
--- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/gpu/drm/tilcdc/tilcdc_drv.c +2 -0
@@ -112,6 +112,8 @@ static int tilcdc_unload(struct drm_device *dev) {struct tilcdc_drm_private *priv = dev->dev_private; +tilcdc_crtc_dpms (priv->crtc,DRM_MODE_DPMS_OFF ); +tilcdc_remove_external_encoders (dev);drm_fbdev_cma_fini (priv->fbdev);
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 12/22 drm/tilcdc: split reset to a separate function
In reply to Jyri SarhaFrom: Tomi Valkeinen <[email protected]>
Split reset to a separate function and use usleep_range(250, 1000) instead of msleep(1) to to keep the reset bit on long enough.
--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +14 -7
@@ -112,17 +112,24 @@ static void update_scanout(struct drm_crtc *crtc) } } -static void start (struct drm_crtc *crtc) +static void reset (struct drm_crtc *crtc) {struct drm_device *dev = crtc->dev;struct tilcdc_drm_private *priv = dev->dev_private; -if (priv->rev ==2 ) { -tilcdc_set (dev,LCDC_CLK_RESET_REG ,LCDC_CLK_MAIN_RESET ); -msleep (1 ); -tilcdc_clear (dev,LCDC_CLK_RESET_REG ,LCDC_CLK_MAIN_RESET ); -msleep (1 ); - } +if (priv->rev !=2 ) +return ; + +tilcdc_set (dev,LCDC_CLK_RESET_REG ,LCDC_CLK_MAIN_RESET ); +usleep_range (250 ,1000 ); +tilcdc_clear (dev,LCDC_CLK_RESET_REG ,LCDC_CLK_MAIN_RESET ); +} + +static void start (struct drm_crtc *crtc) +{ +struct drm_device *dev = crtc->dev; + +reset (crtc);tilcdc_set (dev,LCDC_DMA_CTRL_REG ,LCDC_DUAL_FRAME_BUFFER_ENABLE );tilcdc_set (dev,LCDC_RASTER_CTRL_REG ,LCDC_PALETTE_LOAD_MODE (DATA_ONLY ));
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 13/22 drm/tilcdc: remove broken error handling
In reply to Jyri SarhaFrom: Tomi Valkeinen <[email protected]>
Remove broken error handling. The condition for handling the LCDC_SYNC_LOST and LCDC_FIFO_UNDERFLOW could never be satisfied as the LCDC_SYNC_LOST interrupt is not enabled. Also the requirement to have both LCDC_SYNC_LOST and LCDC_FIFO_UNDERFLOW fired at once before handling the error looks weird.
--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +1 -6
@@ -658,12 +658,7 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)struct tilcdc_drm_private *priv = dev->dev_private;uint32_t stat =tilcdc_read_irqstatus (dev); -if ((stat &LCDC_SYNC_LOST ) && (stat &LCDC_FIFO_UNDERFLOW )) { -stop (crtc); -dev_err (dev->dev,"error: %08x\n" , stat); -tilcdc_clear_irqstatus (dev, stat); -start (crtc); - }else if (stat &LCDC_PL_LOAD_DONE ) { +if (stat &LCDC_PL_LOAD_DONE ) {tilcdc_clear_irqstatus (dev, stat); }else {struct drm_pending_vblank_event *event;
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 14/22 drm/tilcdc: cleanup irq handling
In reply to Jyri SarhaFrom: Tomi Valkeinen <[email protected]>
Cleanup irq handling. Clear the irq status unconditionally and restructure the status bit conditions.
--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +5 -4
@@ -656,11 +656,12 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)struct tilcdc_crtc *tilcdc_crtc =to_tilcdc_crtc (crtc);struct drm_device *dev = crtc->dev;struct tilcdc_drm_private *priv = dev->dev_private; -uint32_t stat =tilcdc_read_irqstatus (dev); +uint32_t stat; -if (stat &LCDC_PL_LOAD_DONE ) { -tilcdc_clear_irqstatus (dev, stat); - }else { + stat =tilcdc_read_irqstatus (dev); +tilcdc_clear_irqstatus (dev, stat); + +if ((stat &LCDC_END_OF_FRAME0 ) || (stat &LCDC_END_OF_FRAME1 )) {struct drm_pending_vblank_event *event;unsigned long flags;uint32_t dirty = tilcdc_crtc->dirty & stat;
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 15/22 drm/tilcdc: Get rid of complex ping-pong mechanism
In reply to Jyri SarhaFrom: Tomi Valkeinen <[email protected]>
Get rid of complex ping-pong mechanism and replace it with simpler single buffer flipping code.
The LCDC HW appears to be designed mainly static framebuffers in mind. There are two modes of operation, either static single buffer, or ping pong double buffering with two static buffers switching back and forth. Luckily the framebuffer start address is fetched only in the beginning of the frame and changing the address after that only takes effect after the next vertical blank. The page flipping code can simply write the address of the new framebuffer and the page is flipped automatically after the next vertical blank. Using the ping pong double buffering makes the flipping code way more complex and it does not provide any benefit, so it is better to switch to single buffer operation.
There is still one problem in updating the framebuffer dma address on the fly. There are two registers defining the framebuffer dma area and things may break if the dma address is fetched in while the registers are are being updated.
--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 121 +++++++++++++++-------------------- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 27 +------- 2 files changed, 53 insertions(+), 95 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +50 -71
@@ -25,15 +25,12 @@ 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 ]; +struct drm_framebuffer *curr_fb;/* for deferred fb unref's: */ struct drm_flip_work unref_work;
@@ -54,62 +51,31 @@ static void unref_worker(struct drm_flip_work *work, void *val)mutex_unlock (&dev->mode_config.mutex); } -static void set_scanout (struct drm_crtc *crtc,int n) -{ -static const uint32_t base_reg[] = { -LCDC_DMA_FB_BASE_ADDR_0_REG , -LCDC_DMA_FB_BASE_ADDR_1_REG , - }; -static const uint32_t ceil_reg[] = { -LCDC_DMA_FB_CEILING_ADDR_0_REG , -LCDC_DMA_FB_CEILING_ADDR_1_REG , - }; -static const uint32_t stat[] = { -LCDC_END_OF_FRAME0 ,LCDC_END_OF_FRAME1 , - }; -struct tilcdc_crtc *tilcdc_crtc =to_tilcdc_crtc (crtc); -struct drm_device *dev = crtc->dev; -struct tilcdc_drm_private *priv = dev->dev_private; - -tilcdc_write (dev, base_reg[n], tilcdc_crtc->start); -tilcdc_write (dev, ceil_reg[n], tilcdc_crtc->end); -if (tilcdc_crtc->scanout[n]) { -drm_flip_work_queue (&tilcdc_crtc->unref_work, tilcdc_crtc->scanout[n]); -drm_flip_work_commit (&tilcdc_crtc->unref_work, priv->wq); - } - tilcdc_crtc->scanout[n] = crtc->primary->fb; -drm_framebuffer_reference (tilcdc_crtc->scanout[n]); - tilcdc_crtc->dirty &= ~stat[n]; -} - -static void update_scanout (struct drm_crtc *crtc) +static void set_scanout (struct drm_crtc *crtc,struct drm_framebuffer *fb) {struct tilcdc_crtc *tilcdc_crtc =to_tilcdc_crtc (crtc);struct drm_device *dev = crtc->dev; -struct drm_framebuffer *fb = crtc->primary->fb;struct drm_gem_cma_object *gem;unsigned int depth, bpp; +dma_addr_t start, end;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 ); + 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 ]); + end = 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 ); - } +tilcdc_write (dev,LCDC_DMA_FB_BASE_ADDR_0_REG , start); +tilcdc_write (dev,LCDC_DMA_FB_CEILING_ADDR_0_REG , end); + +if (tilcdc_crtc->curr_fb) +drm_flip_work_queue (&tilcdc_crtc->unref_work, + tilcdc_crtc->curr_fb); + + tilcdc_crtc->curr_fb = fb; }static void reset (struct drm_crtc *crtc)
@@ -131,7 +97,7 @@ static void start(struct drm_crtc *crtc)reset (crtc); -tilcdc_set (dev,LCDC_DMA_CTRL_REG ,LCDC_DUAL_FRAME_BUFFER_ENABLE ); +tilcdc_clear (dev,LCDC_DMA_CTRL_REG ,LCDC_DUAL_FRAME_BUFFER_ENABLE );tilcdc_set (dev,LCDC_RASTER_CTRL_REG ,LCDC_PALETTE_LOAD_MODE (DATA_ONLY ));tilcdc_set (dev,LCDC_RASTER_CTRL_REG ,LCDC_RASTER_ENABLE ); }
@@ -179,6 +145,7 @@ static int tilcdc_crtc_page_flip(struct drm_crtc *crtc,struct tilcdc_crtc *tilcdc_crtc =to_tilcdc_crtc (crtc);struct drm_device *dev = crtc->dev;int r; +unsigned long flags; r =tilcdc_verify_fb (crtc, fb);if (r)
@@ -189,12 +156,18 @@ static int tilcdc_crtc_page_flip(struct drm_crtc *crtc,return -EBUSY ; } +drm_framebuffer_reference (fb); + crtc->primary->fb = fb; - tilcdc_crtc->event = event;pm_runtime_get_sync (dev->dev); -update_scanout (crtc); + +set_scanout (crtc, fb); + +spin_lock_irqsave (&dev->event_lock, flags); + tilcdc_crtc->event = event; +spin_unlock_irqrestore (&dev->event_lock, flags);pm_runtime_put_sync (dev->dev);
@@ -237,6 +210,14 @@ void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode) }pm_runtime_put_sync (dev->dev); + +if (tilcdc_crtc->curr_fb) { +drm_flip_work_queue (&tilcdc_crtc->unref_work, + tilcdc_crtc->curr_fb); + tilcdc_crtc->curr_fb =NULL ; + } + +drm_flip_work_commit (&tilcdc_crtc->unref_work, priv->wq); } }
@@ -450,8 +431,10 @@ static int tilcdc_crtc_mode_set(struct drm_crtc *crtc,else tilcdc_clear (dev,LCDC_RASTER_CTRL_REG ,LCDC_RASTER_ORDER ); +drm_framebuffer_reference (crtc->primary->fb); + +set_scanout (crtc, crtc->primary->fb); -update_scanout (crtc);tilcdc_crtc_update_clk (crtc);pm_runtime_put_sync (dev->dev);
@@ -469,9 +452,14 @@ static int tilcdc_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,if (r)return r; +drm_framebuffer_reference (crtc->primary->fb); +pm_runtime_get_sync (dev->dev); -update_scanout (crtc); + +set_scanout (crtc, crtc->primary->fb); +pm_runtime_put_sync (dev->dev); +return 0 ; }
@@ -661,30 +649,21 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) stat =tilcdc_read_irqstatus (dev);tilcdc_clear_irqstatus (dev, stat); -if ((stat &LCDC_END_OF_FRAME0 ) || (stat &LCDC_END_OF_FRAME1 )) { -struct drm_pending_vblank_event *event; +if (stat &LCDC_END_OF_FRAME0 ) {unsigned long flags; -uint32_t dirty = tilcdc_crtc->dirty & stat; - -tilcdc_clear_irqstatus (dev, stat); -if (dirty &LCDC_END_OF_FRAME0 ) -set_scanout (crtc,0 ); - -if (dirty &LCDC_END_OF_FRAME1 ) -set_scanout (crtc,1 ); +drm_flip_work_commit (&tilcdc_crtc->unref_work, priv->wq);drm_handle_vblank (dev,0 );spin_lock_irqsave (&dev->event_lock, flags); - event = tilcdc_crtc->event; - tilcdc_crtc->event =NULL ; -if (event) -drm_send_vblank_event (dev,0 , event); -spin_unlock_irqrestore (&dev->event_lock, flags); -if (dirty && !tilcdc_crtc->dirty) -drm_vblank_put (dev,0 ); +if (tilcdc_crtc->event) { +drm_send_vblank_event (dev,0 , tilcdc_crtc->event); + tilcdc_crtc->event =NULL ; + } + +spin_unlock_irqrestore (&dev->event_lock, flags); }if (priv->rev ==2 ) {
drivers/gpu/drm/tilcdc/tilcdc_drv.c +3 -24
@@ -381,6 +381,7 @@ static int tilcdc_irq_postinstall(struct drm_device *dev)else tilcdc_set (dev,LCDC_INT_ENABLE_SET_REG ,LCDC_V2_UNDERFLOW_INT_ENA | +LCDC_V2_END_OF_FRAME0_INT_ENA |LCDC_FRAME_DONE );return 0 ;
@@ -398,41 +399,19 @@ static void tilcdc_irq_uninstall(struct drm_device *dev) }else {tilcdc_clear (dev,LCDC_INT_ENABLE_SET_REG ,LCDC_V2_UNDERFLOW_INT_ENA |LCDC_V2_PL_INT_ENA | -LCDC_V2_END_OF_FRAME0_INT_ENA |LCDC_V2_END_OF_FRAME1_INT_ENA | +LCDC_V2_END_OF_FRAME0_INT_ENA |LCDC_FRAME_DONE ); } - -} - -static void enable_vblank (struct drm_device *dev,bool enable) -{ -struct tilcdc_drm_private *priv = dev->dev_private; -u32 reg, mask; - -if (priv->rev ==1 ) { - reg =LCDC_DMA_CTRL_REG ; - mask =LCDC_V1_END_OF_FRAME_INT_ENA ; - }else { - reg =LCDC_INT_ENABLE_SET_REG ; - mask =LCDC_V2_END_OF_FRAME0_INT_ENA | -LCDC_V2_END_OF_FRAME1_INT_ENA ; - } - -if (enable) -tilcdc_set (dev, reg, mask); -else -tilcdc_clear (dev, reg, mask); }static int tilcdc_enable_vblank (struct drm_device *dev,unsigned int pipe) { -enable_vblank (dev, true);return 0 ; }static void tilcdc_disable_vblank (struct drm_device *dev,unsigned int pipe) { -enable_vblank (dev, false); +return ; }#if defined(CONFIG_DEBUG_FS ) || defined(CONFIG_PM_SLEEP )
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 16/22 drm/tilcdc: Do not update the next frame buffer close to vertical blank
In reply to Jyri SarhaFrom: Tomi Valkeinen <[email protected]>
Do not update the next frame buffer close to vertical blank. This is to avoid situation when the frame changes between writing of LCDC_DMA_FB_BASE_ADDR_0_REG and LCDC_DMA_FB_CEILING_ADDR_0_REG.
--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 61 +++++++++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 8 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +53 -8
@@ -21,6 +21,8 @@#include "tilcdc_drv.h" #include "tilcdc_regs.h" +#define TILCDC_VBLANK_SAFETY_THRESHOLD_US 1000 +struct tilcdc_crtc {struct drm_crtc base;
@@ -29,8 +31,12 @@ struct tilcdc_crtc {int dpms;wait_queue_head_t frame_done_wq;bool frame_done; +spinlock_t irq_lock; + +ktime_t last_vblank;struct drm_framebuffer *curr_fb; +struct drm_framebuffer *next_fb;/* for deferred fb unref's: */ struct drm_flip_work unref_work;
@@ -146,6 +152,8 @@ static int tilcdc_crtc_page_flip(struct drm_crtc *crtc,struct drm_device *dev = crtc->dev;int r;unsigned long flags; +s64 tdiff; +ktime_t next_vblank; r =tilcdc_verify_fb (crtc, fb);if (r)
@@ -162,12 +170,21 @@ static int tilcdc_crtc_page_flip(struct drm_crtc *crtc,pm_runtime_get_sync (dev->dev); +spin_lock_irqsave (&tilcdc_crtc->irq_lock, flags); + + next_vblank =ktime_add_us (tilcdc_crtc->last_vblank, +1000000 / crtc->hwmode.vrefresh); -set_scanout (crtc, fb); + tdiff =ktime_to_us (ktime_sub (next_vblank,ktime_get ())); + +if (tdiff >=TILCDC_VBLANK_SAFETY_THRESHOLD_US ) +set_scanout (crtc, fb); +else + tilcdc_crtc->next_fb = fb; -spin_lock_irqsave (&dev->event_lock, flags); tilcdc_crtc->event = event; -spin_unlock_irqrestore (&dev->event_lock, flags); + +spin_unlock_irqrestore (&tilcdc_crtc->irq_lock, flags);pm_runtime_put_sync (dev->dev);
@@ -211,6 +228,12 @@ void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode)pm_runtime_put_sync (dev->dev); +if (tilcdc_crtc->next_fb) { +drm_flip_work_queue (&tilcdc_crtc->unref_work, + tilcdc_crtc->next_fb); + tilcdc_crtc->next_fb =NULL ; + } +if (tilcdc_crtc->curr_fb) {drm_flip_work_queue (&tilcdc_crtc->unref_work, tilcdc_crtc->curr_fb);
@@ -651,19 +674,39 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)if (stat &LCDC_END_OF_FRAME0 ) {unsigned long flags; +bool skip_event = false; +ktime_t now; + + now =ktime_get ();drm_flip_work_commit (&tilcdc_crtc->unref_work, priv->wq); +spin_lock_irqsave (&tilcdc_crtc->irq_lock, flags); + + tilcdc_crtc->last_vblank = now; + +if (tilcdc_crtc->next_fb) { +set_scanout (crtc, tilcdc_crtc->next_fb); + tilcdc_crtc->next_fb =NULL ; + skip_event = true; + } + +spin_unlock_irqrestore (&tilcdc_crtc->irq_lock, flags); +drm_handle_vblank (dev,0 ); -spin_lock_irqsave (&dev->event_lock, flags); +if (!skip_event) { +struct drm_pending_vblank_event *event; -if (tilcdc_crtc->event) { -drm_send_vblank_event (dev,0 , tilcdc_crtc->event); +spin_lock_irqsave (&dev->event_lock, flags); + + event = tilcdc_crtc->event; tilcdc_crtc->event =NULL ; - } +if (event) +drm_send_vblank_event (dev,0 , event); -spin_unlock_irqrestore (&dev->event_lock, flags); +spin_unlock_irqrestore (&dev->event_lock, flags); + } }if (priv->rev ==2 ) {
@@ -697,6 +740,8 @@ struct drm_crtc *tilcdc_crtc_create(struct drm_device *dev)drm_flip_work_init (&tilcdc_crtc->unref_work,"unref" , unref_worker); +spin_lock_init (&tilcdc_crtc->irq_lock); + ret =drm_crtc_init (dev, crtc, &tilcdc_crtc_funcs);if (ret <0 ) goto fail;
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 17/22 drm/tilcdc: Fix interrupt enable/disable code for version 2 tilcdc
In reply to Jyri SarhaFix interrupt enable/disable code for version 2 tilcdc. In version 2 tilcdc there is a separate register for disabling interrupts. Writing 0 to enable registers bits does not have any effect. The interrupt clear register works the same way, writing 1 to specific bit disables the interrupt and writing 0 does not have any effect.
The "bug" that is fixed here does not really do any harm since the interrupts are enabled only once in the power up and disabled before power down.
--- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_drv.c +6 -5
@@ -376,13 +376,14 @@ static int tilcdc_irq_postinstall(struct drm_device *dev)struct tilcdc_drm_private *priv = dev->dev_private;/* enable FIFO underflow irq: */ -if (priv->rev ==1 ) +if (priv->rev ==1 ) {tilcdc_set (dev,LCDC_RASTER_CTRL_REG ,LCDC_V1_UNDERFLOW_INT_ENA ); -else -tilcdc_set (dev,LCDC_INT_ENABLE_SET_REG , + }else { +tilcdc_write (dev,LCDC_INT_ENABLE_SET_REG ,LCDC_V2_UNDERFLOW_INT_ENA |LCDC_V2_END_OF_FRAME0_INT_ENA |LCDC_FRAME_DONE ); + }return 0 ; }
@@ -397,7 +398,7 @@ static void tilcdc_irq_uninstall(struct drm_device *dev) LCDC_V1_UNDERFLOW_INT_ENA | LCDC_V1_PL_INT_ENA);tilcdc_clear (dev,LCDC_DMA_CTRL_REG ,LCDC_V1_END_OF_FRAME_INT_ENA ); }else { -tilcdc_clear (dev,LCDC_INT_ENABLE_SET_REG , +tilcdc_write (dev,LCDC_INT_ENABLE_CLR_REG ,LCDC_V2_UNDERFLOW_INT_ENA |LCDC_V2_PL_INT_ENA |LCDC_V2_END_OF_FRAME0_INT_ENA |LCDC_FRAME_DONE );
@@ -442,7 +443,7 @@ static const struct {REG (2 , false,LCDC_INT_ENABLE_CLR_REG ),REG (2 , false,LCDC_END_OF_INT_IND_REG ),REG (2 , true,LCDC_CLK_ENABLE_REG ), -REG (2 , true,LCDC_INT_ENABLE_SET_REG ), +REG (2 , true,LCDC_INT_ENABLE_SET_REG ), #undef REG };
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 18/22 drm/tilcdc: Remove the duplicate LCDC_INT_ENABLE_SET_REG in registers[]
In reply to Jyri SarhaRemoves the duplicate LCDC_INT_ENABLE_SET_REG-entry in registers array.
--- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_drv.c +1 -2
@@ -439,11 +439,10 @@ static const struct {/* new in revision 2: */ REG (2 , false,LCDC_RAW_STAT_REG ),REG (2 , false,LCDC_MASKED_STAT_REG ), -REG (2 , false,LCDC_INT_ENABLE_SET_REG ), +REG (2 , true,LCDC_INT_ENABLE_SET_REG ),REG (2 , false,LCDC_INT_ENABLE_CLR_REG ),REG (2 , false,LCDC_END_OF_INT_IND_REG ),REG (2 , true,LCDC_CLK_ENABLE_REG ), -REG (2 , true,LCDC_INT_ENABLE_SET_REG ), #undef REG };
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 19/22 drm/tilcdc: Add prints on sync lost and FIFO underrun interrupts
In reply to Jyri SarhaAdd ratelimited prints on sync lost and FIFO underrun interrupts.
--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 8 ++++++++ drivers/gpu/drm/tilcdc/tilcdc_drv.c | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +8 -0
@@ -717,6 +717,14 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)tilcdc_write (dev,LCDC_END_OF_INT_IND_REG ,0 ); } +if (stat &LCDC_SYNC_LOST ) +dev_err_ratelimited (dev->dev,"%s(0x%08x): Sync lost" , + __func__, stat); + +if (stat &LCDC_FIFO_UNDERFLOW ) +dev_err_ratelimited (dev->dev,"%s(0x%08x): FIFO underfow" , + __func__, stat); +return IRQ_HANDLED ; }
drivers/gpu/drm/tilcdc/tilcdc_drv.c +2 -2
@@ -382,7 +382,7 @@ static int tilcdc_irq_postinstall(struct drm_device *dev)tilcdc_write (dev,LCDC_INT_ENABLE_SET_REG ,LCDC_V2_UNDERFLOW_INT_ENA |LCDC_V2_END_OF_FRAME0_INT_ENA | -LCDC_FRAME_DONE ); +LCDC_FRAME_DONE |LCDC_SYNC_LOST ); }return 0 ;
@@ -401,7 +401,7 @@ static void tilcdc_irq_uninstall(struct drm_device *dev)tilcdc_write (dev,LCDC_INT_ENABLE_CLR_REG ,LCDC_V2_UNDERFLOW_INT_ENA |LCDC_V2_PL_INT_ENA |LCDC_V2_END_OF_FRAME0_INT_ENA | -LCDC_FRAME_DONE ); +LCDC_FRAME_DONE |LCDC_SYNC_LOST ); } }
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 20/22 drm/tilcdc: Disable sync lost interrupt if it fires on every frame
In reply to Jyri SarhaDisable the sync lost interrupt if it fires on every frame for 50 consecutive frames in a row. This is relatively sure sign of the sync lost interrupt being stuck and firing on every frame even if the display otherwise appears to work OK.
--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +20 -1
@@ -43,6 +43,9 @@ struct tilcdc_crtc {/* Only set if an external encoder is connected */ bool simulate_vesa_sync; + +int sync_lost_count; +bool frame_intact; };#define to_tilcdc_crtc (x) container_of(x, struct tilcdc_crtc, base)
@@ -662,6 +665,8 @@ out:pm_runtime_put_sync (dev->dev); } +#define SYNC_LOST_COUNT_LIMIT 50 +irqreturn_t tilcdc_crtc_irq (struct drm_crtc *crtc) {struct tilcdc_crtc *tilcdc_crtc =to_tilcdc_crtc (crtc);
@@ -707,6 +712,11 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)spin_unlock_irqrestore (&dev->event_lock, flags); } + +if (tilcdc_crtc->frame_intact) + tilcdc_crtc->sync_lost_count =0 ; +else + tilcdc_crtc->frame_intact = true; }if (priv->rev ==2 ) {
@@ -717,9 +727,18 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)tilcdc_write (dev,LCDC_END_OF_INT_IND_REG ,0 ); } -if (stat &LCDC_SYNC_LOST ) +if (stat &LCDC_SYNC_LOST ) {dev_err_ratelimited (dev->dev,"%s(0x%08x): Sync lost" , __func__, stat); + tilcdc_crtc->frame_intact = false; +if (tilcdc_crtc->sync_lost_count++ >SYNC_LOST_COUNT_LIMIT ) { +dev_err (dev->dev, +"%s(0x%08x): Sync lost flood detected, disabling the interrupt" , + __func__, stat); +tilcdc_write (dev,LCDC_INT_ENABLE_CLR_REG , +LCDC_SYNC_LOST ); + } + }if (stat &LCDC_FIFO_UNDERFLOW ) dev_err_ratelimited(dev->dev, "%s(0x%08x): FIFO underfow",
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 21/22 drm/tilcdc: Initialize crtc->port
In reply to Jyri SarhaInitialize port device node pointer in the tilcdc crtc. Fixes "Falling back to first CRTC" warning from tda998x driver.
The tda998x encoder driver calls drm_of_find_possible_crtcs() to initialize possible_crtcs of struct drm_encoder. The crtc->port needs to be initialized for drm_of_find_possible_crtcs() to work.
--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +20 -0
@@ -124,6 +124,7 @@ static void tilcdc_crtc_destroy(struct drm_crtc *crtc)tilcdc_crtc_dpms (crtc,DRM_MODE_DPMS_OFF ); +of_node_put (crtc->port);drm_crtc_cleanup (crtc);drm_flip_work_cleanup (&tilcdc_crtc->unref_work);
@@ -749,6 +750,7 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)struct drm_crtc *tilcdc_crtc_create (struct drm_device *dev) { +struct tilcdc_drm_private *priv = dev->dev_private;struct tilcdc_crtc *tilcdc_crtc;struct drm_crtc *crtc;int ret;
@@ -775,6 +777,24 @@ struct drm_crtc *tilcdc_crtc_create(struct drm_device *dev)drm_crtc_helper_add (crtc, &tilcdc_crtc_helper_funcs); +if (priv->is_componentized) { +struct device_node *ports = +of_get_child_by_name (dev->dev->of_node,"ports" ); + +if (ports) { + crtc->port =of_get_child_by_name (ports,"port" ); +of_node_put (ports); + }else { + crtc->port = +of_get_child_by_name (dev->dev->of_node,"port" ); + } +if (!crtc->port) {/* This should never happen */ +dev_err (dev->dev,"Port node not found in %s\n" , + dev->dev->of_node->full_name); + goto fail; + } + } +return crtc; fail:
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v4 22/22 drm/tilcdc: Use devm_kzalloc() and devm_kcalloc() for private data
In reply to Jyri SarhaUse devm_kzalloc() and devm_kcalloc() for private data allocation at driver load time.
--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 4 +--- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 19 +++++++------------ drivers/gpu/drm/tilcdc/tilcdc_panel.c | 20 ++++++-------------- drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 24 +++++++----------------- 4 files changed, 21 insertions(+), 46 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +1 -3
@@ -127,8 +127,6 @@ static void tilcdc_crtc_destroy(struct drm_crtc *crtc)of_node_put (crtc->port);drm_crtc_cleanup (crtc);drm_flip_work_cleanup (&tilcdc_crtc->unref_work); - -kfree (tilcdc_crtc); }static int tilcdc_verify_fb (struct drm_crtc *crtc,struct drm_framebuffer *fb)
@@ -755,7 +753,7 @@ struct drm_crtc *tilcdc_crtc_create(struct drm_device *dev)struct drm_crtc *crtc;int ret; - tilcdc_crtc =kzalloc (sizeof (*tilcdc_crtc),GFP_KERNEL ); + tilcdc_crtc =devm_kzalloc (dev->dev,sizeof (*tilcdc_crtc),GFP_KERNEL );if (!tilcdc_crtc) {dev_err (dev->dev,"allocation failed\n" );return NULL ;
drivers/gpu/drm/tilcdc/tilcdc_drv.c +7 -12
@@ -143,9 +143,6 @@ static int tilcdc_unload(struct drm_device *dev)pm_runtime_disable (dev->dev); -kfree (priv->saved_register); -kfree (priv); -return 0 ; }
@@ -161,13 +158,12 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)u32 bpp =0 ;int ret; - priv =kzalloc (sizeof (*priv),GFP_KERNEL ); + priv =devm_kzalloc (dev->dev,sizeof (*priv),GFP_KERNEL );if (priv) - priv->saved_register =kcalloc (tilcdc_num_regs (), -sizeof (*priv->saved_register), -GFP_KERNEL ); + priv->saved_register = +devm_kcalloc (dev->dev,tilcdc_num_regs (), +sizeof (*priv->saved_register),GFP_KERNEL );if (!priv || !priv->saved_register) { -kfree (priv);dev_err (dev->dev,"failed to allocate private data\n" );return -ENOMEM ; }
@@ -180,7 +176,7 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags) priv->wq =alloc_ordered_workqueue ("tilcdc" ,0 );if (!priv->wq) { ret = -ENOMEM ; - goto fail_free_priv; + goto fail_unset_priv; } res =platform_get_resource (pdev,IORESOURCE_MEM ,0 );
@@ -346,10 +342,9 @@ fail_free_wq:flush_workqueue (priv->wq);destroy_workqueue (priv->wq); -fail_free_priv: +fail_unset_priv: dev->dev_private =NULL ; -kfree (priv->saved_register); -kfree (priv); +return ret; }
drivers/gpu/drm/tilcdc/tilcdc_panel.c +6 -14
@@ -45,14 +45,6 @@ struct panel_encoder { };#define to_panel_encoder (x) container_of(x, struct panel_encoder, base) - -static void panel_encoder_destroy (struct drm_encoder *encoder) -{ -struct panel_encoder *panel_encoder =to_panel_encoder (encoder); -drm_encoder_cleanup (encoder); -kfree (panel_encoder); -} -static void panel_encoder_dpms (struct drm_encoder *encoder,int mode) {struct panel_encoder *panel_encoder =to_panel_encoder (encoder);
@@ -90,7 +82,7 @@ static void panel_encoder_mode_set(struct drm_encoder *encoder, }static const struct drm_encoder_funcs panel_encoder_funcs = { - .destroy = panel_encoder_destroy, + .destroy = drm_encoder_cleanup, };static const struct drm_encoder_helper_funcs panel_encoder_helper_funcs = {
@@ -107,7 +99,8 @@ static struct drm_encoder *panel_encoder_create(struct drm_device *dev,struct drm_encoder *encoder;int ret; - panel_encoder =kzalloc (sizeof (*panel_encoder),GFP_KERNEL ); + panel_encoder =devm_kzalloc (dev->dev,sizeof (*panel_encoder), +GFP_KERNEL );if (!panel_encoder) {dev_err (dev->dev,"allocation failed\n" );return NULL ;
@@ -128,7 +121,7 @@ static struct drm_encoder *panel_encoder_create(struct drm_device *dev,return encoder; fail: -panel_encoder_destroy (encoder); +drm_encoder_cleanup (encoder);return NULL ; }
@@ -147,10 +140,8 @@ struct panel_connector {static void panel_connector_destroy (struct drm_connector *connector) { -struct panel_connector *panel_connector =to_panel_connector (connector);drm_connector_unregister (connector);drm_connector_cleanup (connector); -kfree (panel_connector); } static enum drm_connector_status panel_connector_detect(
@@ -223,7 +214,8 @@ static struct drm_connector *panel_connector_create(struct drm_device *dev,struct drm_connector *connector;int ret; - panel_connector =kzalloc (sizeof (*panel_connector),GFP_KERNEL ); + panel_connector =devm_kzalloc (dev->dev,sizeof (*panel_connector), +GFP_KERNEL );if (!panel_connector) {dev_err (dev->dev,"allocation failed\n" );return NULL ;
drivers/gpu/drm/tilcdc/tilcdc_tfp410.c +7 -17
@@ -54,14 +54,6 @@ struct tfp410_encoder { };#define to_tfp410_encoder (x) container_of(x, struct tfp410_encoder, base) - -static void tfp410_encoder_destroy (struct drm_encoder *encoder) -{ -struct tfp410_encoder *tfp410_encoder =to_tfp410_encoder (encoder); -drm_encoder_cleanup (encoder); -kfree (tfp410_encoder); -} -static void tfp410_encoder_dpms (struct drm_encoder *encoder,int mode) {struct tfp410_encoder *tfp410_encoder =to_tfp410_encoder (encoder);
@@ -99,7 +91,7 @@ static void tfp410_encoder_mode_set(struct drm_encoder *encoder, }static const struct drm_encoder_funcs tfp410_encoder_funcs = { - .destroy = tfp410_encoder_destroy, + .destroy = drm_encoder_cleanup, };static const struct drm_encoder_helper_funcs tfp410_encoder_helper_funcs = {
@@ -116,7 +108,8 @@ static struct drm_encoder *tfp410_encoder_create(struct drm_device *dev,struct drm_encoder *encoder;int ret; - tfp410_encoder =kzalloc (sizeof (*tfp410_encoder),GFP_KERNEL ); + tfp410_encoder =devm_kzalloc (dev->dev,sizeof (*tfp410_encoder), +GFP_KERNEL );if (!tfp410_encoder) {dev_err (dev->dev,"allocation failed\n" );return NULL ;
@@ -138,7 +131,7 @@ static struct drm_encoder *tfp410_encoder_create(struct drm_device *dev,return encoder; fail: -tfp410_encoder_destroy (encoder); +drm_encoder_cleanup (encoder);return NULL ; }
@@ -157,10 +150,8 @@ struct tfp410_connector {static void tfp410_connector_destroy (struct drm_connector *connector) { -struct tfp410_connector *tfp410_connector =to_tfp410_connector (connector);drm_connector_unregister (connector);drm_connector_cleanup (connector); -kfree (tfp410_connector); } static enum drm_connector_status tfp410_connector_detect(
@@ -228,7 +219,8 @@ static struct drm_connector *tfp410_connector_create(struct drm_device *dev,struct drm_connector *connector;int ret; - tfp410_connector =kzalloc (sizeof (*tfp410_connector),GFP_KERNEL ); + tfp410_connector =devm_kzalloc (dev->dev,sizeof (*tfp410_connector), +GFP_KERNEL );if (!tfp410_connector) {dev_err (dev->dev,"allocation failed\n" );return NULL ;
@@ -313,7 +305,7 @@ static int tfp410_probe(struct platform_device *pdev)return -ENXIO ; } - tfp410_mod =kzalloc (sizeof (*tfp410_mod),GFP_KERNEL ); + tfp410_mod =devm_kzalloc (&pdev->dev,sizeof (*tfp410_mod),GFP_KERNEL );if (!tfp410_mod)return -ENOMEM ;
@@ -366,7 +358,6 @@ fail_adapter:i2c_put_adapter (tfp410_mod->i2c); fail: -kfree (tfp410_mod);tilcdc_module_cleanup (mod);return ret; }
@@ -380,7 +371,6 @@ static int tfp410_remove(struct platform_device *pdev)gpio_free (tfp410_mod->gpio);tilcdc_module_cleanup (mod); -kfree (tfp410_mod);return 0 ; }
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 15/22] drm/tilcdc: Get rid of complex ping-pong mechanism
In reply to Jyri SarhaQuoted 17 lines · Jyri Sarha
From: Tomi Valkeinen <[email protected]>
Get rid of complex ping-pong mechanism and replace it with simpler single buffer flipping code.
The LCDC HW appears to be designed mainly static framebuffers in mind. There are two modes of operation, either static single buffer, or ping pong double buffering with two static buffers switching back and forth. Luckily the framebuffer start address is fetched only in the beginning of the frame and changing the address after that only takes effect after the next vertical blank. The page flipping code can simply write the address of the new framebuffer and the page is flipped automatically after the next vertical blank. Using the ping pong double buffering makes the flipping code way more complex and it does not provide any benefit, so it is better to switch to single buffer operation.
Quoted 4 lines
There is still one problem in updating the framebuffer dma address on the fly. There are two registers defining the framebuffer dma area and things may break if the dma address is fetched in while the registers are are being updated.
Just curious, but does this mean you need to avoid writing the reg's too near to vblank? (I think there are other drivers which do not have double buffered registers and must do this.. but it's a pain..)
The ping-pong mode is super-wonky, and would be nice to get rid of.. but with this scheme I'm not quite sure what happens if a pageflip comes in too close to vblank..
BR,
-R
Quoted 283 lines
Signed-off-by Tomi Valkeinen <[email protected]>[Added description to the patch]Signed-off-by Jyri Sarha <[email protected]>--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 121 +++++++++++++++-------------------- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 27 +------- 2 files changed, 53 insertions(+), 95 deletions(-)drivers/gpu/drm/tilcdc/tilcdc_crtc.c +50
-71@@ -25,15 +25,12 @@ 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 ]; +struct drm_framebuffer *curr_fb;/* for deferred fb unref's: */ struct drm_flip_work unref_work;@@ -54,62 +51,31 @@ static void unref_worker(struct drm_flip_work *work, void *val)mutex_unlock (&dev->mode_config.mutex); } -static void set_scanout (struct drm_crtc *crtc,int n) -{ -static const uint32_t base_reg[] = { -LCDC_DMA_FB_BASE_ADDR_0_REG , -LCDC_DMA_FB_BASE_ADDR_1_REG , - }; -static const uint32_t ceil_reg[] = { -LCDC_DMA_FB_CEILING_ADDR_0_REG , -LCDC_DMA_FB_CEILING_ADDR_1_REG , - }; -static const uint32_t stat[] = { -LCDC_END_OF_FRAME0 ,LCDC_END_OF_FRAME1 , - }; -struct tilcdc_crtc *tilcdc_crtc =to_tilcdc_crtc (crtc); -struct drm_device *dev = crtc->dev; -struct tilcdc_drm_private *priv = dev->dev_private; - -tilcdc_write (dev, base_reg[n], tilcdc_crtc->start); -tilcdc_write (dev, ceil_reg[n], tilcdc_crtc->end); -if (tilcdc_crtc->scanout[n]) { -drm_flip_work_queue (&tilcdc_crtc->unref_work, tilcdc_crtc->scanout[n]); -drm_flip_work_commit (&tilcdc_crtc->unref_work, priv->wq); - } - tilcdc_crtc->scanout[n] = crtc->primary->fb; -drm_framebuffer_reference (tilcdc_crtc->scanout[n]); - tilcdc_crtc->dirty &= ~stat[n]; -} - -static void update_scanout (struct drm_crtc *crtc) +static void set_scanout (struct drm_crtc *crtc,struct drm_framebuffer *fb) {struct tilcdc_crtc *tilcdc_crtc =to_tilcdc_crtc (crtc);struct drm_device *dev = crtc->dev; -struct drm_framebuffer *fb = crtc->primary->fb;struct drm_gem_cma_object *gem;unsigned int depth, bpp; +dma_addr_t start, end;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 ); + 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 ]); + end = 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 ); - } +tilcdc_write (dev,LCDC_DMA_FB_BASE_ADDR_0_REG , start); +tilcdc_write (dev,LCDC_DMA_FB_CEILING_ADDR_0_REG , end); + +if (tilcdc_crtc->curr_fb) +drm_flip_work_queue (&tilcdc_crtc->unref_work, + tilcdc_crtc->curr_fb); + + tilcdc_crtc->curr_fb = fb; }static void reset (struct drm_crtc *crtc)@@ -131,7 +97,7 @@ static void start(struct drm_crtc *crtc)reset (crtc); -tilcdc_set (dev,LCDC_DMA_CTRL_REG ,LCDC_DUAL_FRAME_BUFFER_ENABLE ); +tilcdc_clear (dev,LCDC_DMA_CTRL_REG ,LCDC_DUAL_FRAME_BUFFER_ENABLE );tilcdc_set (dev,LCDC_RASTER_CTRL_REG ,LCDC_PALETTE_LOAD_MODE (DATA_ONLY ));tilcdc_set (dev,LCDC_RASTER_CTRL_REG ,LCDC_RASTER_ENABLE ); }@@ -179,6 +145,7 @@ static int tilcdc_crtc_page_flip(struct drm_crtc *crtc,struct tilcdc_crtc *tilcdc_crtc =to_tilcdc_crtc (crtc);struct drm_device *dev = crtc->dev;int r; +unsigned long flags; r =tilcdc_verify_fb (crtc, fb);if (r)@@ -189,12 +156,18 @@ static int tilcdc_crtc_page_flip(struct drm_crtc *crtc,return -EBUSY ; } +drm_framebuffer_reference (fb); + crtc->primary->fb = fb; - tilcdc_crtc->event = event;pm_runtime_get_sync (dev->dev); -update_scanout (crtc); + +set_scanout (crtc, fb); + +spin_lock_irqsave (&dev->event_lock, flags); + tilcdc_crtc->event = event; +spin_unlock_irqrestore (&dev->event_lock, flags);pm_runtime_put_sync (dev->dev);@@ -237,6 +210,14 @@ void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode) }pm_runtime_put_sync (dev->dev); + +if (tilcdc_crtc->curr_fb) { +drm_flip_work_queue (&tilcdc_crtc->unref_work, + tilcdc_crtc->curr_fb); + tilcdc_crtc->curr_fb =NULL ; + } + +drm_flip_work_commit (&tilcdc_crtc->unref_work, priv->wq); } }@@ -450,8 +431,10 @@ static int tilcdc_crtc_mode_set(struct drm_crtc *crtc,else tilcdc_clear (dev,LCDC_RASTER_CTRL_REG ,LCDC_RASTER_ORDER ); +drm_framebuffer_reference (crtc->primary->fb); + +set_scanout (crtc, crtc->primary->fb); -update_scanout (crtc);tilcdc_crtc_update_clk (crtc);pm_runtime_put_sync (dev->dev);@@ -469,9 +452,14 @@ static int tilcdc_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,if (r)return r; +drm_framebuffer_reference (crtc->primary->fb); +pm_runtime_get_sync (dev->dev); -update_scanout (crtc); + +set_scanout (crtc, crtc->primary->fb); +pm_runtime_put_sync (dev->dev); +return 0 ; }@@ -661,30 +649,21 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) stat =tilcdc_read_irqstatus (dev);tilcdc_clear_irqstatus (dev, stat); -if ((stat &LCDC_END_OF_FRAME0 ) || (stat &LCDC_END_OF_FRAME1 )) { -struct drm_pending_vblank_event *event; +if (stat &LCDC_END_OF_FRAME0 ) {unsigned long flags; -uint32_t dirty = tilcdc_crtc->dirty & stat; - -tilcdc_clear_irqstatus (dev, stat); -if (dirty &LCDC_END_OF_FRAME0 ) -set_scanout (crtc,0 ); - -if (dirty &LCDC_END_OF_FRAME1 ) -set_scanout (crtc,1 ); +drm_flip_work_commit (&tilcdc_crtc->unref_work, priv->wq);drm_handle_vblank (dev,0 );spin_lock_irqsave (&dev->event_lock, flags); - event = tilcdc_crtc->event; - tilcdc_crtc->event =NULL ; -if (event) -drm_send_vblank_event (dev,0 , event); -spin_unlock_irqrestore (&dev->event_lock, flags); -if (dirty && !tilcdc_crtc->dirty) -drm_vblank_put (dev,0 ); +if (tilcdc_crtc->event) { +drm_send_vblank_event (dev,0 , tilcdc_crtc->event); + tilcdc_crtc->event =NULL ; + } + +spin_unlock_irqrestore (&dev->event_lock, flags); }if (priv->rev ==2 ) {drivers/gpu/drm/tilcdc/tilcdc_drv.c +3
-25@@ -381,6 +381,7 @@ static int tilcdc_irq_postinstall(struct drm_device *dev)else tilcdc_set (dev,LCDC_INT_ENABLE_SET_REG ,LCDC_V2_UNDERFLOW_INT_ENA | +LCDC_V2_END_OF_FRAME0_INT_ENA |LCDC_FRAME_DONE );return 0 ;@@ -398,41 +399,19 @@ static void tilcdc_irq_uninstall(struct drm_device *dev) }else {tilcdc_clear (dev,LCDC_INT_ENABLE_SET_REG ,LCDC_V2_UNDERFLOW_INT_ENA |LCDC_V2_PL_INT_ENA | -LCDC_V2_END_OF_FRAME0_INT_ENA |LCDC_V2_END_OF_FRAME1_INT_ENA | +LCDC_V2_END_OF_FRAME0_INT_ENA |LCDC_FRAME_DONE ); } - -} - -static void enable_vblank (struct drm_device *dev,bool enable) -{ -struct tilcdc_drm_private *priv = dev->dev_private; -u32 reg, mask; - -if (priv->rev ==1 ) { - reg =LCDC_DMA_CTRL_REG ; - mask =LCDC_V1_END_OF_FRAME_INT_ENA ; - }else { - reg =LCDC_INT_ENABLE_SET_REG ; - mask =LCDC_V2_END_OF_FRAME0_INT_ENA | -LCDC_V2_END_OF_FRAME1_INT_ENA ; - } - -if (enable) -tilcdc_set (dev, reg, mask); -else -tilcdc_clear (dev, reg, mask); }static int tilcdc_enable_vblank (struct drm_device *dev,unsigned int pipe) { -enable_vblank (dev, true);return 0 ; }static void tilcdc_disable_vblank (struct drm_device *dev,unsigned int pipe) { -enable_vblank (dev, false); +return ; }#if defined(CONFIG_DEBUG_FS ) || defined(CONFIG_PM_SLEEP ) --
Quoted 2 lines
1.9.1
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 15/22] drm/tilcdc: Get rid of complex ping-pong mechanism
Quoted 18 lines · Rob Clark
Quoted 17 lines · Jyri Sarha
From: Tomi Valkeinen <[email protected]>
Get rid of complex ping-pong mechanism and replace it with simpler single buffer flipping code.
The LCDC HW appears to be designed mainly static framebuffers in mind. There are two modes of operation, either static single buffer, or ping pong double buffering with two static buffers switching back and forth. Luckily the framebuffer start address is fetched only in the beginning of the frame and changing the address after that only takes effect after the next vertical blank. The page flipping code can simply write the address of the new framebuffer and the page is flipped automatically after the next vertical blank. Using the ping pong double buffering makes the flipping code way more complex and it does not provide any benefit, so it is better to switch to single buffer operation.
Quoted 7 lines
Quoted 4 lines
There is still one problem in updating the framebuffer dma address on the fly. There are two registers defining the framebuffer dma area and things may break if the dma address is fetched in while the registers are are being updated.
Just curious, but does this mean you need to avoid writing the reg's too near to vblank? (I think there are other drivers which do not
Yes.
Quoted 5 lines
have double buffered registers and must do this.. but it's a pain..)
The ping-pong mode is super-wonky, and would be nice to get rid of.. but with this scheme I'm not quite sure what happens if a pageflip comes in too close to vblank..
It's the same problem with both single and double buffer (ping-pong) modes. The HW is designed to have a static configuration of one or two buffers, never changed at runtime. If you change the buffers at runtime, and change them exactly at the bad time during vblank when the HW starts using the start and end addresses, you end up with broken display.
In other words, using the double buffering mode only adds extra complexity to the driver, without any benefit. In fact, with the double-buffering mode I think it's impossible to make the system reliable. The driver cannot reliably track which of those two buffers are currently shown on the screen, so when you do a page flip, you could flip the wrong buffer.
The next patch adds some safety margin for the "page-flip near vblank" issue.
Tomi
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi,
Quoted 1 line · Jyri Sarha
... and yet one more round.
For the series:
Tomi
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 15/22] drm/tilcdc: Get rid of complex ping-pong mechanism
In reply to Tomi ValkeinenQuoted 50 lines · Tomi Valkeinen
Quoted 18 lines · Rob Clark
Quoted 17 lines · Jyri Sarha
From: Tomi Valkeinen <[email protected]>
Get rid of complex ping-pong mechanism and replace it with simpler single buffer flipping code.
The LCDC HW appears to be designed mainly static framebuffers in mind. There are two modes of operation, either static single buffer, or ping pong double buffering with two static buffers switching back and forth. Luckily the framebuffer start address is fetched only in the beginning of the frame and changing the address after that only takes effect after the next vertical blank. The page flipping code can simply write the address of the new framebuffer and the page is flipped automatically after the next vertical blank. Using the ping pong double buffering makes the flipping code way more complex and it does not provide any benefit, so it is better to switch to single buffer operation.
Quoted 7 lines
Quoted 4 lines
There is still one problem in updating the framebuffer dma address on the fly. There are two registers defining the framebuffer dma area and things may break if the dma address is fetched in while the registers are are being updated.
Just curious, but does this mean you need to avoid writing the reg's too near to vblank? (I think there are other drivers which do not
Yes.
Quoted 5 lines
have double buffered registers and must do this.. but it's a pain..)
The ping-pong mode is super-wonky, and would be nice to get rid of.. but with this scheme I'm not quite sure what happens if a pageflip comes in too close to vblank..
It's the same problem with both single and double buffer (ping-pong) modes. The HW is designed to have a static configuration of one or two buffers, never changed at runtime. If you change the buffers at runtime, and change them exactly at the bad time during vblank when the HW starts using the start and end addresses, you end up with broken display.
In other words, using the double buffering mode only adds extra complexity to the driver, without any benefit. In fact, with the double-buffering mode I think it's impossible to make the system reliable. The driver cannot reliably track which of those two buffers are currently shown on the screen, so when you do a page flip, you could flip the wrong buffer.
Quoted 2 lines
The next patch adds some safety margin for the "page-flip near vblank" issue.
ahh, gotcha, I should have looked at the complete patchset
BR,
-R
Quoted 2 lines
Tomi
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel