drm/tilcdc Fixes and cleanups
Subsystems (4)
v2 cover drm/tilcdc Fixes and cleanups
Some fixes and cleanups that should get merged to tilcdc even if my atomic changes are still a work in progress.
Jyri Sarha (6):
drm/tilcdc: Restore old dpms state in pm_resume() drm/tilcdc: Write to LCDC_END_OF_INT_IND_REG at the end of IRQ
function
drm/tilcdc: Move waiting of LCDC_FRAME_DONE IRQ into stop() drm/tilcdc: Call drm_crtc_vblank_on() and *_off() in start() and
stop()
drm/tilcdc: Refer to panel.txt and tfp410.txt bindings in tilcdc.txt drm/tilcdc: Avoid error print by of_graph_get_next_endpoint()
.../devicetree/bindings/display/tilcdc/tilcdc.txt | 4 ++ drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 66 ++++++++++++++-------- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 3 + drivers/gpu/drm/tilcdc/tilcdc_drv.h | 2 + drivers/gpu/drm/tilcdc/tilcdc_external.c | 13 ++++- 5 files changed, 62 insertions(+), 26 deletions(-)
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v2 1/6 drm/tilcdc: Restore old dpms state in pm_resume()
Restore old dpms state in pm_resume(). The dpms is turned off in pm_suspend() and it should be restored to its original state in pm_resume().
Fixes commit 614b3cfeb8d2 ("drm/tilcdc: disable the lcd controller/dma engine when suspend invoked")
--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 7 +++++++ drivers/gpu/drm/tilcdc/tilcdc_drv.c | 3 +++ drivers/gpu/drm/tilcdc/tilcdc_drv.h | 2 ++ 3 files changed, 12 insertions(+)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +7 -0
@@ -246,6 +246,13 @@ void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode) } } +int tilcdc_crtc_current_dpms_state (struct drm_crtc *crtc) +{ +struct tilcdc_crtc *tilcdc_crtc =to_tilcdc_crtc (crtc); + +return tilcdc_crtc->dpms; +} +static bool tilcdc_crtc_mode_fixup (struct drm_crtc *crtc,const struct drm_display_mode *mode,struct drm_display_mode *adjusted_mode)
drivers/gpu/drm/tilcdc/tilcdc_drv.c +3 -0
@@ -598,6 +598,7 @@ static int tilcdc_pm_suspend(struct device *dev) }/* Disable the LCDC controller, to avoid locking up the PRCM */ + priv->saved_dpms_state =tilcdc_crtc_current_dpms_state (priv->crtc);tilcdc_crtc_dpms (priv->crtc,DRM_MODE_DPMS_OFF );/* Save register state: */
@@ -628,6 +629,8 @@ static int tilcdc_pm_resume(struct device *dev) priv->saved_register[n++]); } +tilcdc_crtc_dpms (priv->crtc, priv->saved_dpms_state); +drm_kms_helper_poll_enable (ddev);return 0 ;
drivers/gpu/drm/tilcdc/tilcdc_drv.h +2 -0
@@ -67,6 +67,7 @@ struct tilcdc_drm_private {/* register contents saved across suspend/resume: */ u32 *saved_register; +int saved_dpms_state;bool ctx_valid;#ifdef CONFIG_CPU_FREQ
@@ -172,5 +173,6 @@ void tilcdc_crtc_set_simulate_vesa_sync(struct drm_crtc *crtc,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); +int tilcdc_crtc_current_dpms_state (struct drm_crtc *crtc);#endif /* __TILCDC_DRV_H__ */
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/6] drm/tilcdc: Restore old dpms state in pm_resume()
Quoted 6 lines · Jyri Sarha
Restore old dpms state in
pm_resume(). The dpms is turned off inpm_suspend()and it should be restored to its original state inpm_resume().Fixes commit
614b3cfeb8d2("drm/tilcdc: disable the lcd controller/dma engine when suspend invoked")
This still doesn't explain the issue (at least clearly). If you send a fix for some problem, it's good to have a clear explanation of what's the problem (from user's perspective, what does he see).
Tomi
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v2 2/6 drm/tilcdc: Write to LCDC_END_OF_INT_IND_REG at the end of IRQ function
In reply to Jyri SarhaReorder the IRQ function so that the write to LCDC_END_OF_INT_IND_REG is done last. The write to LCDC_END_OF_INT_IND_REG indicates to LCDC that the interrupt service routine has completed (see section 13.3.6.1.6 in AM335x TRM). This is needed if LCDC's ipgvmodirq module is configured for pulse interrupts.
--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +15 -9
@@ -725,14 +725,19 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) tilcdc_crtc->frame_intact = true; } -if (priv->rev ==2 ) { -if (stat &LCDC_FRAME_DONE ) { - tilcdc_crtc->frame_done = true; -wake_up (&tilcdc_crtc->frame_done_wq); - } -tilcdc_write (dev,LCDC_END_OF_INT_IND_REG ,0 ); +if (priv->rev ==1 ) +return IRQ_HANDLED ; +/* The rest is for revision 2 only */ + +if (stat &LCDC_FRAME_DONE ) { + tilcdc_crtc->frame_done = true; +wake_up (&tilcdc_crtc->frame_done_wq); } +if (stat &LCDC_FIFO_UNDERFLOW ) +dev_err_ratelimited (dev->dev,"%s(0x%08x): FIFO underfow" , + __func__, stat); +if (stat &LCDC_SYNC_LOST ) {dev_err_ratelimited (dev->dev,"%s(0x%08x): Sync lost" , __func__, stat);
@@ -746,9 +751,10 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) } } -if (stat &LCDC_FIFO_UNDERFLOW ) -dev_err_ratelimited (dev->dev,"%s(0x%08x): FIFO underfow" , - __func__, stat); +/* Indicate to LCDC that the interrupt service routine has +* completed, see 13.3.6.1.6 in AM335x TRM. +*/ +tilcdc_write (dev,LCDC_END_OF_INT_IND_REG ,0 );return IRQ_HANDLED ; }
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/6] drm/tilcdc: Write to LCDC_END_OF_INT_IND_REG at the end of IRQ function
Quoted 27 lines · Jyri Sarha
Reorder the IRQ function so that the write to
LCDC_END_OF_INT_IND_REGis done last. The write toLCDC_END_OF_INT_IND_REGindicates to LCDC that the interrupt service routine has completed (see section 13.3.6.1.6 in AM335x TRM). This is needed if LCDC's ipgvmodirq module is configured for pulse interrupts.Signed-off-by Jyri Sarha <[email protected]>--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)drivers/gpu/drm/tilcdc/tilcdc_crtc.c +2
-6@@ -725,14 +725,19 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) tilcdc_crtc->frame_intact = true; } -if (priv->rev ==2 ) { -if (stat &LCDC_FRAME_DONE ) { - tilcdc_crtc->frame_done = true; -wake_up (&tilcdc_crtc->frame_done_wq); - } -tilcdc_write (dev,LCDC_END_OF_INT_IND_REG ,0 ); +if (priv->rev ==1 ) +return IRQ_HANDLED ;
drivers/gpu/drm/tilcdc/tilcdc_crtc.c irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) · line 739
Quoted 10 lines
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +8
-0+/* The rest is for revision 2 only */ + +if (stat &LCDC_FRAME_DONE ) { + tilcdc_crtc->frame_done = true; +wake_up (&tilcdc_crtc->frame_done_wq); } +if (stat &LCDC_FIFO_UNDERFLOW ) +dev_err_ratelimited (dev->dev,"%s(0x%08x): FIFO underfow" , + __func__, stat);
We do have underflow irq for rev1 too, don't we?
Why not just move the "if (priv->rev == 2) {" block to the end? Or maybe extract the write to the LCDC_END_OF_INT_IND_REG from the current block, and move only that to the end. Much less re-ordering needed for that.
Tomi
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v2 3/6 drm/tilcdc: Move waiting of LCDC_FRAME_DONE IRQ into stop()
In reply to Jyri SarhaMove wait queue waiting of LCDC_FRAME_DONE IRQ from tilcdc_crtc_dpms() into stop() function.
--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +16 -15
@@ -113,9 +113,25 @@ static void start(struct drm_crtc *crtc)static void stop (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; + tilcdc_crtc->frame_done = false;tilcdc_clear (dev,LCDC_RASTER_CTRL_REG ,LCDC_RASTER_ENABLE ); + +/* +* if necessary wait for framedone irq which will still come +* before putting things to sleep.. +*/ +if (priv->rev ==2 ) { +int ret =wait_event_timeout (tilcdc_crtc->frame_done_wq, + tilcdc_crtc->frame_done, +msecs_to_jiffies (50 )); +if (ret ==0 ) +dev_err (dev->dev,"%s: timeout waiting for framedone\n" , + __func__); + } }static void tilcdc_crtc_destroy (struct drm_crtc *crtc)
@@ -212,22 +228,7 @@ void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode)pm_runtime_get_sync (dev->dev);start (crtc); }else { - tilcdc_crtc->frame_done = false;stop (crtc); - -/* -* if necessary wait for framedone irq which will still come -* before putting things to sleep.. -*/ -if (priv->rev ==2 ) { -int ret =wait_event_timeout ( - tilcdc_crtc->frame_done_wq, - tilcdc_crtc->frame_done, -msecs_to_jiffies (50 )); -if (ret ==0 ) -dev_err (dev->dev,"timeout waiting for framedone\n" ); - } -pm_runtime_put_sync (dev->dev);if (tilcdc_crtc->next_fb) {
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/6] drm/tilcdc: Move waiting of LCDC_FRAME_DONE IRQ into stop()
Quoted 2 lines · Jyri Sarha
Move wait queue waiting of
LCDC_FRAME_DONEIRQ fromtilcdc_crtc_dpms()intostop()function.
You could mention the reason for this, which is just a cleanup here (I presume).
Quoted 18 lines
Signed-off-by Jyri Sarha <[email protected]>--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-)drivers/gpu/drm/tilcdc/tilcdc_crtc.c +3
-0@@ -113,9 +113,25 @@ static void start(struct drm_crtc *crtc)static void stop (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; + tilcdc_crtc->frame_done = false;
drivers/gpu/drm/tilcdc/tilcdc_crtc.c static void start(struct drm_crtc *crtc) · line 130
Quoted 10 lines
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +9
-0tilcdc_clear (dev,LCDC_RASTER_CTRL_REG ,LCDC_RASTER_ENABLE ); + +/* +* if necessary wait for framedone irq which will still come +* before putting things to sleep.. +*/ +if (priv->rev ==2 ) { +int ret =wait_event_timeout (tilcdc_crtc->frame_done_wq, + tilcdc_crtc->frame_done, +msecs_to_jiffies (50 ));
Not part of this patch, but I think the timeout could be increased quite a bit. It's never supposed to happen. And while video timings with 50ms frames are not common, I don't see that as impossible either.
Of course, optimally the timeout would be calculated from the video timings, which shouldn't be too difficult either...
Anyway, as I said, not part of this patch.
Tomi
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v2 4/6 drm/tilcdc: Call drm_crtc_vblank_on() and *_off() in start() and stop()
In reply to Jyri SarhaAdd drm_crtc_vblank_on() and *_off() calls to start() and stop() functions, to make sure any vblank waits etc. gets properly cleaned up.
--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 4 ++++ 1 file changed, 4 insertions(+)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +4 -0
@@ -109,6 +109,8 @@ static void start(struct drm_crtc *crtc)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 ); + +drm_crtc_vblank_on (crtc); }static void stop (struct drm_crtc *crtc)
@@ -132,6 +134,8 @@ static void stop(struct drm_crtc *crtc)dev_err (dev->dev,"%s: timeout waiting for framedone\n" , __func__); } + +drm_crtc_vblank_off (crtc); }static void tilcdc_crtc_destroy (struct drm_crtc *crtc)
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v2 5/6 drm/tilcdc: Refer to panel.txt and tfp410.txt bindings in tilcdc.txt
In reply to Jyri SarhaThe legacy panel.txt and tfp410.txt bindings are still the only supported way to connect lcd panel and tfp410 DVI encoder to tilcdc.
--- Documentation/devicetree/bindings/display/tilcdc/tilcdc.txt | 4 ++++ 1 file changed, 4 insertions(+)
Documentation/devicetree/bindings/display/tilcdc/tilcdc.txt +4 -0
@@ -24,6 +24,10 @@ Optional nodes: binding follows Documentation/devicetree/bindings/graph.txt and suppors a single port with a single endpoint. + - See also Documentation/devicetree/bindings/display/tilcdc/panel.txt and + Documentation/devicetree/bindings/display/tilcdc/tfp410.txt for connecting + tfp410 DVI encoder or lcd panel to lcdc + Example: fb: fb@4830e000 {
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v2 6/6 drm/tilcdc: Avoid error print by of_graph_get_next_endpoint()
In reply to Jyri SarhaAvoid error print by of_graph_get_next_endpoint() if there is no ports present.
--- drivers/gpu/drm/tilcdc/tilcdc_external.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_external.c +11 -2
@@ -138,12 +138,21 @@ static int dev_match_of(struct device *dev, void *data)int tilcdc_get_external_components (struct device *dev,struct component_match **match) { +struct device_node *node;struct device_node *ep =NULL ;int count =0 ; -while ((ep =of_graph_get_next_endpoint (dev->of_node, ep))) { -struct device_node *node; +/* Avoid error print by of_graph_get_next_endpoint() if there +* is no ports present. +*/ + node =of_get_child_by_name (dev->of_node,"ports" ); +if (!node) + node =of_get_child_by_name (dev->of_node,"port" ); +if (!node) +return 0 ; +of_node_put (node); +while ((ep =of_graph_get_next_endpoint (dev->of_node, ep))) { node =of_graph_get_remote_port_parent (ep);if (!node && !of_device_is_available (node)) {of_node_put (node);
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoted 3 lines · Jyri Sarha
Some fixes and cleanups that should get merged to tilcdc even if my atomic changes are still a work in progress.
I forgot to add changelog. So here it is:
Changes since first version:
- "drm/tilcdc: Restore old dpms state in pm_resume()"
- Fix typos from description and subject
- Add "drm/tilcdc: Call drm_crtc_vblank_on() and *_off() in start() and stop()"
BR,
Jyri
Quoted 10 lines
Jyri Sarha (6):
drm/tilcdc: Restore old dpms state inpm_resume()drm/tilcdc: Write toLCDC_END_OF_INT_IND_REGat the end of IRQ
function
drm/tilcdc: Move waiting ofLCDC_FRAME_DONEIRQ intostop()drm/tilcdc: Calldrm_crtc_vblank_on()and*_off()instart()andstop()drm/tilcdc: Refer to panel.txt and tfp410.txt bindings in tilcdc.txt drm/tilcdc: Avoid error print by
of_graph_get_next_endpoint()
Quoted 7 lines
.../devicetree/bindings/display/tilcdc/tilcdc.txt | 4 ++ drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 66 ++++++++++++++-------- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 3 + drivers/gpu/drm/tilcdc/tilcdc_drv.h | 2 + drivers/gpu/drm/tilcdc/tilcdc_external.c | 13 ++++- 5 files changed, 62 insertions(+), 26 deletions(-)
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoted 13 lines · Jyri Sarha
Some fixes and cleanups that should get merged to tilcdc even if my atomic changes are still a work in progress.
Jyri Sarha (6):
drm/tilcdc: Restore old dpms state inpm_resume()drm/tilcdc: Write toLCDC_END_OF_INT_IND_REGat the end of IRQ
function
drm/tilcdc: Move waiting ofLCDC_FRAME_DONEIRQ intostop()drm/tilcdc: Calldrm_crtc_vblank_on()and*_off()instart()andstop()drm/tilcdc: Refer to panel.txt and tfp410.txt bindings in tilcdc.txt drm/tilcdc: Avoid error print by
of_graph_get_next_endpoint()
Quoted 7 lines
.../devicetree/bindings/display/tilcdc/tilcdc.txt | 4 ++ drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 66 ++++++++++++++-------- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 3 + drivers/gpu/drm/tilcdc/tilcdc_drv.h | 2 + drivers/gpu/drm/tilcdc/tilcdc_external.c | 13 ++++- 5 files changed, 62 insertions(+), 26 deletions(-)
Beside the minor comments I had, looks fine to me.
Tomi
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel