* [PATCH 0/2] Making Exynos MIPI Complaint with Common Display Framework
@ 2013-01-02 13:17 Vikas C Sajjan
2013-01-02 13:17 ` [PATCH 1/2] [RFC] video: exynos mipi dsi: Making Exynos MIPI Complaint with CDF Vikas C Sajjan
2013-01-02 13:17 ` [PATCH 2/2] [RFC] video: display: Adding frame related ops to MIPI DSI video source struct Vikas C Sajjan
0 siblings, 2 replies; 11+ messages in thread
From: Vikas C Sajjan @ 2013-01-02 13:17 UTC (permalink / raw)
To: linux-media, dri-devel
Cc: inki.dae, laurent.pinchart, tomi.valkeinen, jesse.barker,
aditya.ps
From: Vikas Sajjan <vikas.sajjan@linaro.org>
This patchset contains 2 RFCs, 1st RFC has modiifications in exynos MIPI DSI
driver.
2nd RFC has additions done to video source struct as per exynos requirements.
I have NOT tested the patch yet, as i am yet recieve the MIPI DSI panel.
This based on Tomi's CDF RFC.
I am yet to modify s6e8ax0.c as per CDF and I have NOT tested these patches yet,
as i am yet recieve the MIPI DSI panel.
Vikas Sajjan (2):
[RFC] video: exynos mipi dsi: Making Exynos MIPI Complaint with CDF
[RFC] video: display: Adding frame related ops to MIPI DSI video
source struct
drivers/video/exynos/exynos_mipi_dsi.c | 46 ++++++++++++++++++-------
drivers/video/exynos/exynos_mipi_dsi_common.c | 22 ++++++++----
drivers/video/exynos/exynos_mipi_dsi_common.h | 12 +++----
include/video/display.h | 6 ++++
include/video/exynos_mipi_dsim.h | 5 ++-
5 files changed, 62 insertions(+), 29 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/2] [RFC] video: exynos mipi dsi: Making Exynos MIPI Complaint with CDF 2013-01-02 13:17 [PATCH 0/2] Making Exynos MIPI Complaint with Common Display Framework Vikas C Sajjan @ 2013-01-02 13:17 ` Vikas C Sajjan 2013-01-03 5:44 ` Sachin Kamat 2013-01-02 13:17 ` [PATCH 2/2] [RFC] video: display: Adding frame related ops to MIPI DSI video source struct Vikas C Sajjan 1 sibling, 1 reply; 11+ messages in thread From: Vikas C Sajjan @ 2013-01-02 13:17 UTC (permalink / raw) To: linux-media, dri-devel Cc: inki.dae, laurent.pinchart, tomi.valkeinen, jesse.barker, aditya.ps From: Vikas Sajjan <vikas.sajjan@linaro.org> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org> --- drivers/video/exynos/exynos_mipi_dsi.c | 46 ++++++++++++++++++------- drivers/video/exynos/exynos_mipi_dsi_common.c | 22 ++++++++---- drivers/video/exynos/exynos_mipi_dsi_common.h | 12 +++---- include/video/exynos_mipi_dsim.h | 5 ++- 4 files changed, 56 insertions(+), 29 deletions(-) diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c index 07d70a3..88b2aa9 100644 --- a/drivers/video/exynos/exynos_mipi_dsi.c +++ b/drivers/video/exynos/exynos_mipi_dsi.c @@ -32,14 +32,13 @@ #include <linux/notifier.h> #include <linux/regulator/consumer.h> #include <linux/pm_runtime.h> - +#include <video/display.h> #include <video/exynos_mipi_dsim.h> #include <plat/fb.h> #include "exynos_mipi_dsi_common.h" #include "exynos_mipi_dsi_lowlevel.h" - struct mipi_dsim_ddi { int bus_id; struct list_head list; @@ -111,12 +110,13 @@ static void exynos_mipi_update_cfg(struct mipi_dsim_device *dsim) exynos_mipi_dsi_stand_by(dsim, 1); } -static int exynos_mipi_dsi_early_blank_mode(struct mipi_dsim_device *dsim, +static int exynos_mipi_dsi_early_blank_mode(struct video_source *video_source, int power) { + struct mipi_dsim_device *dsim = container_of(video_source, + struct mipi_dsim_device, video_source); struct mipi_dsim_lcd_driver *client_drv = dsim->dsim_lcd_drv; struct mipi_dsim_lcd_device *client_dev = dsim->dsim_lcd_dev; - switch (power) { case FB_BLANK_POWERDOWN: if (dsim->suspended) @@ -139,12 +139,13 @@ static int exynos_mipi_dsi_early_blank_mode(struct mipi_dsim_device *dsim, return 0; } -static int exynos_mipi_dsi_blank_mode(struct mipi_dsim_device *dsim, int power) +static int exynos_mipi_dsi_blank_mode(struct video_source *video_source, int power) { + struct mipi_dsim_device *dsim = container_of(video_source, + struct mipi_dsim_device, video_source); struct platform_device *pdev = to_platform_device(dsim->dev); struct mipi_dsim_lcd_driver *client_drv = dsim->dsim_lcd_drv; struct mipi_dsim_lcd_device *client_dev = dsim->dsim_lcd_dev; - switch (power) { case FB_BLANK_UNBLANK: if (!dsim->suspended) @@ -319,12 +320,19 @@ static struct mipi_dsim_ddi *exynos_mipi_dsi_bind_lcd_ddi( return NULL; } -/* define MIPI-DSI Master operations. */ -static struct mipi_dsim_master_ops master_ops = { - .cmd_read = exynos_mipi_dsi_rd_data, - .cmd_write = exynos_mipi_dsi_wr_data, - .get_dsim_frame_done = exynos_mipi_dsi_get_frame_done_status, - .clear_dsim_frame_done = exynos_mipi_dsi_clear_frame_done, +static void panel_dsi_release(struct video_source *src) +{ + printk("dsi entity release\n"); +} + +static const struct common_video_source_ops dsi_common_ops = { +}; + +static const struct dsi_video_source_ops exynos_dsi_ops = { + .dcs_read = exynos_mipi_dsi_rd_data, + .dcs_write = exynos_mipi_dsi_wr_data, + .get_frame_done = exynos_mipi_dsi_get_frame_done_status, + .clear_frame_done = exynos_mipi_dsi_clear_frame_done, .set_early_blank_mode = exynos_mipi_dsi_early_blank_mode, .set_blank_mode = exynos_mipi_dsi_blank_mode, }; @@ -362,7 +370,6 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev) } dsim->dsim_config = dsim_config; - dsim->master_ops = &master_ops; mutex_init(&dsim->lock); @@ -463,6 +470,19 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev) dsim->suspended = false; + dsim->video_source.dev = &pdev->dev; + dsim->video_source.release = panel_dsi_release; + dsim->video_source.common_ops = &dsi_common_ops; + dsim->video_source.ops.dsi = &exynos_dsi_ops; + dsim->video_source.name = "exynos"; + + ret = video_source_register(&dsim->video_source); + if (ret < 0) { + printk("dsi entity register failed\n"); + goto err_bind; + } + printk("dsi entity registered: %p\n", &dsim->video_source); + return 0; done: platform_set_drvdata(pdev, dsim); diff --git a/drivers/video/exynos/exynos_mipi_dsi_common.c b/drivers/video/exynos/exynos_mipi_dsi_common.c index 3cd29a4..e59911e 100644 --- a/drivers/video/exynos/exynos_mipi_dsi_common.c +++ b/drivers/video/exynos/exynos_mipi_dsi_common.c @@ -153,11 +153,12 @@ static void exynos_mipi_dsi_long_data_wr(struct mipi_dsim_device *dsim, } } } - -int exynos_mipi_dsi_wr_data(struct mipi_dsim_device *dsim, unsigned int data_id, - const unsigned char *data0, unsigned int data_size) +int exynos_mipi_dsi_wr_data(struct video_source *video_source, int data_id, + u8 *data0, size_t data_size) { unsigned int check_rx_ack = 0; + struct mipi_dsim_device *dsim = container_of(video_source, + struct mipi_dsim_device, video_source); if (dsim->state == DSIM_STATE_ULPS) { dev_err(dsim->dev, "state is ULPS.\n"); @@ -340,12 +341,14 @@ static unsigned int exynos_mipi_dsi_response_size(unsigned int req_size) } } -int exynos_mipi_dsi_rd_data(struct mipi_dsim_device *dsim, unsigned int data_id, - unsigned int data0, unsigned int req_size, u8 *rx_buf) +int exynos_mipi_dsi_rd_data(struct video_source *video_source, int data_id, + u8 data0, u8 *rx_buf,size_t req_size) { unsigned int rx_data, rcv_pkt, i; u8 response = 0; u16 rxsize; + struct mipi_dsim_device *dsim = container_of(video_source, + struct mipi_dsim_device, video_source); if (dsim->state == DSIM_STATE_ULPS) { dev_err(dsim->dev, "state is ULPS.\n"); @@ -843,13 +846,18 @@ int exynos_mipi_dsi_set_data_transfer_mode(struct mipi_dsim_device *dsim, return 0; } -int exynos_mipi_dsi_get_frame_done_status(struct mipi_dsim_device *dsim) +int exynos_mipi_dsi_get_frame_done_status(struct video_source *video_source) { + struct mipi_dsim_device *dsim = container_of(video_source, + struct mipi_dsim_device, video_source); + return _exynos_mipi_dsi_get_frame_done_status(dsim); } -int exynos_mipi_dsi_clear_frame_done(struct mipi_dsim_device *dsim) +int exynos_mipi_dsi_clear_frame_done(struct video_source *video_source) { + struct mipi_dsim_device *dsim = container_of(video_source, + struct mipi_dsim_device, video_source); _exynos_mipi_dsi_clear_frame_done(dsim); return 0; diff --git a/drivers/video/exynos/exynos_mipi_dsi_common.h b/drivers/video/exynos/exynos_mipi_dsi_common.h index 4125522..cd89154 100644 --- a/drivers/video/exynos/exynos_mipi_dsi_common.h +++ b/drivers/video/exynos/exynos_mipi_dsi_common.h @@ -18,10 +18,10 @@ static DECLARE_COMPLETION(dsim_rd_comp); static DECLARE_COMPLETION(dsim_wr_comp); -int exynos_mipi_dsi_wr_data(struct mipi_dsim_device *dsim, unsigned int data_id, - const unsigned char *data0, unsigned int data_size); -int exynos_mipi_dsi_rd_data(struct mipi_dsim_device *dsim, unsigned int data_id, - unsigned int data0, unsigned int req_size, u8 *rx_buf); +int exynos_mipi_dsi_rd_data(struct video_source *video_source, int data_id, + u8 data0, u8 *rx_buf,size_t req_size); +int exynos_mipi_dsi_wr_data(struct video_source *video_source, int data_id, + u8 *data0, size_t data_size); irqreturn_t exynos_mipi_dsi_interrupt_handler(int irq, void *dev_id); void exynos_mipi_dsi_init_interrupt(struct mipi_dsim_device *dsim); int exynos_mipi_dsi_init_dsim(struct mipi_dsim_device *dsim); @@ -35,8 +35,8 @@ int exynos_mipi_dsi_set_data_transfer_mode(struct mipi_dsim_device *dsim, unsigned int mode); int exynos_mipi_dsi_enable_frame_done_int(struct mipi_dsim_device *dsim, unsigned int enable); -int exynos_mipi_dsi_get_frame_done_status(struct mipi_dsim_device *dsim); -int exynos_mipi_dsi_clear_frame_done(struct mipi_dsim_device *dsim); +int exynos_mipi_dsi_get_frame_done_status(struct video_source *video_source); +int exynos_mipi_dsi_clear_frame_done(struct video_source *video_source); extern struct fb_info *registered_fb[FB_MAX] __read_mostly; diff --git a/include/video/exynos_mipi_dsim.h b/include/video/exynos_mipi_dsim.h index 83ce5e6..e50438e 100644 --- a/include/video/exynos_mipi_dsim.h +++ b/include/video/exynos_mipi_dsim.h @@ -17,7 +17,7 @@ #include <linux/device.h> #include <linux/fb.h> - +#include <video/display.h> #define PANEL_NAME_SIZE (32) /* @@ -225,9 +225,8 @@ struct mipi_dsim_device { unsigned int irq; void __iomem *reg_base; struct mutex lock; - + struct video_source video_source; struct mipi_dsim_config *dsim_config; - struct mipi_dsim_master_ops *master_ops; struct mipi_dsim_lcd_device *dsim_lcd_dev; struct mipi_dsim_lcd_driver *dsim_lcd_drv; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] [RFC] video: exynos mipi dsi: Making Exynos MIPI Complaint with CDF 2013-01-02 13:17 ` [PATCH 1/2] [RFC] video: exynos mipi dsi: Making Exynos MIPI Complaint with CDF Vikas C Sajjan @ 2013-01-03 5:44 ` Sachin Kamat 2013-01-03 8:30 ` Vikas Sajjan 0 siblings, 1 reply; 11+ messages in thread From: Sachin Kamat @ 2013-01-03 5:44 UTC (permalink / raw) To: Vikas C Sajjan Cc: linux-media, dri-devel, inki.dae, laurent.pinchart, tomi.valkeinen, jesse.barker, aditya.ps Hi Vikas, Some nitpicks inline Subject: s/Complaint/Compliant On 2 January 2013 18:47, Vikas C Sajjan <vikas.sajjan@linaro.org> wrote: > From: Vikas Sajjan <vikas.sajjan@linaro.org> > Please add some description about this patch here. > Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org> > --- > drivers/video/exynos/exynos_mipi_dsi.c | 46 ++++++++++++++++++------- > drivers/video/exynos/exynos_mipi_dsi_common.c | 22 ++++++++---- > drivers/video/exynos/exynos_mipi_dsi_common.h | 12 +++---- > include/video/exynos_mipi_dsim.h | 5 ++- > 4 files changed, 56 insertions(+), 29 deletions(-) > > diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c > index 07d70a3..88b2aa9 100644 > --- a/drivers/video/exynos/exynos_mipi_dsi.c > +++ b/drivers/video/exynos/exynos_mipi_dsi.c > @@ -32,14 +32,13 @@ > #include <linux/notifier.h> > #include <linux/regulator/consumer.h> > #include <linux/pm_runtime.h> > - > +#include <video/display.h> > #include <video/exynos_mipi_dsim.h> > > #include <plat/fb.h> > > #include "exynos_mipi_dsi_common.h" > #include "exynos_mipi_dsi_lowlevel.h" > - > struct mipi_dsim_ddi { > int bus_id; > struct list_head list; > @@ -111,12 +110,13 @@ static void exynos_mipi_update_cfg(struct mipi_dsim_device *dsim) > exynos_mipi_dsi_stand_by(dsim, 1); > } > > -static int exynos_mipi_dsi_early_blank_mode(struct mipi_dsim_device *dsim, > +static int exynos_mipi_dsi_early_blank_mode(struct video_source *video_source, > int power) > { > + struct mipi_dsim_device *dsim = container_of(video_source, > + struct mipi_dsim_device, video_source); > struct mipi_dsim_lcd_driver *client_drv = dsim->dsim_lcd_drv; > struct mipi_dsim_lcd_device *client_dev = dsim->dsim_lcd_dev; > - > switch (power) { > case FB_BLANK_POWERDOWN: > if (dsim->suspended) > @@ -139,12 +139,13 @@ static int exynos_mipi_dsi_early_blank_mode(struct mipi_dsim_device *dsim, > return 0; > } > > -static int exynos_mipi_dsi_blank_mode(struct mipi_dsim_device *dsim, int power) > +static int exynos_mipi_dsi_blank_mode(struct video_source *video_source, int power) > { > + struct mipi_dsim_device *dsim = container_of(video_source, > + struct mipi_dsim_device, video_source); > struct platform_device *pdev = to_platform_device(dsim->dev); > struct mipi_dsim_lcd_driver *client_drv = dsim->dsim_lcd_drv; > struct mipi_dsim_lcd_device *client_dev = dsim->dsim_lcd_dev; > - > switch (power) { > case FB_BLANK_UNBLANK: > if (!dsim->suspended) > @@ -319,12 +320,19 @@ static struct mipi_dsim_ddi *exynos_mipi_dsi_bind_lcd_ddi( > return NULL; > } > > -/* define MIPI-DSI Master operations. */ > -static struct mipi_dsim_master_ops master_ops = { > - .cmd_read = exynos_mipi_dsi_rd_data, > - .cmd_write = exynos_mipi_dsi_wr_data, > - .get_dsim_frame_done = exynos_mipi_dsi_get_frame_done_status, > - .clear_dsim_frame_done = exynos_mipi_dsi_clear_frame_done, > +static void panel_dsi_release(struct video_source *src) How about exynos_dsi_panel_release in line with other function names? > +{ > + printk("dsi entity release\n"); Please use pr_* function here (instead of printk). > +} > + > +static const struct common_video_source_ops dsi_common_ops = { Some comments for this place holder would be good. > +}; > + > +static const struct dsi_video_source_ops exynos_dsi_ops = { > + .dcs_read = exynos_mipi_dsi_rd_data, > + .dcs_write = exynos_mipi_dsi_wr_data, > + .get_frame_done = exynos_mipi_dsi_get_frame_done_status, > + .clear_frame_done = exynos_mipi_dsi_clear_frame_done, > .set_early_blank_mode = exynos_mipi_dsi_early_blank_mode, > .set_blank_mode = exynos_mipi_dsi_blank_mode, > }; > @@ -362,7 +370,6 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev) > } > > dsim->dsim_config = dsim_config; > - dsim->master_ops = &master_ops; > > mutex_init(&dsim->lock); > > @@ -463,6 +470,19 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev) > > dsim->suspended = false; > > + dsim->video_source.dev = &pdev->dev; > + dsim->video_source.release = panel_dsi_release; > + dsim->video_source.common_ops = &dsi_common_ops; > + dsim->video_source.ops.dsi = &exynos_dsi_ops; > + dsim->video_source.name = "exynos"; Can't we get this from pdev? > + > + ret = video_source_register(&dsim->video_source); > + if (ret < 0) { > + printk("dsi entity register failed\n"); Please use pr_* function here (instead of printk). > + goto err_bind; > + } > + printk("dsi entity registered: %p\n", &dsim->video_source); ditto. > + return 0; > done: > platform_set_drvdata(pdev, dsim); > > diff --git a/drivers/video/exynos/exynos_mipi_dsi_common.c b/drivers/video/exynos/exynos_mipi_dsi_common.c > index 3cd29a4..e59911e 100644 > --- a/drivers/video/exynos/exynos_mipi_dsi_common.c > +++ b/drivers/video/exynos/exynos_mipi_dsi_common.c > @@ -153,11 +153,12 @@ static void exynos_mipi_dsi_long_data_wr(struct mipi_dsim_device *dsim, > } > } > } > - > -int exynos_mipi_dsi_wr_data(struct mipi_dsim_device *dsim, unsigned int data_id, > - const unsigned char *data0, unsigned int data_size) > +int exynos_mipi_dsi_wr_data(struct video_source *video_source, int data_id, > + u8 *data0, size_t data_size) > { > unsigned int check_rx_ack = 0; > + struct mipi_dsim_device *dsim = container_of(video_source, > + struct mipi_dsim_device, video_source); > > if (dsim->state == DSIM_STATE_ULPS) { > dev_err(dsim->dev, "state is ULPS.\n"); > @@ -340,12 +341,14 @@ static unsigned int exynos_mipi_dsi_response_size(unsigned int req_size) > } > } > > -int exynos_mipi_dsi_rd_data(struct mipi_dsim_device *dsim, unsigned int data_id, > - unsigned int data0, unsigned int req_size, u8 *rx_buf) > +int exynos_mipi_dsi_rd_data(struct video_source *video_source, int data_id, > + u8 data0, u8 *rx_buf,size_t req_size) > { > unsigned int rx_data, rcv_pkt, i; > u8 response = 0; > u16 rxsize; > + struct mipi_dsim_device *dsim = container_of(video_source, > + struct mipi_dsim_device, video_source); > > if (dsim->state == DSIM_STATE_ULPS) { > dev_err(dsim->dev, "state is ULPS.\n"); > @@ -843,13 +846,18 @@ int exynos_mipi_dsi_set_data_transfer_mode(struct mipi_dsim_device *dsim, > return 0; > } > > -int exynos_mipi_dsi_get_frame_done_status(struct mipi_dsim_device *dsim) > +int exynos_mipi_dsi_get_frame_done_status(struct video_source *video_source) > { > + struct mipi_dsim_device *dsim = container_of(video_source, > + struct mipi_dsim_device, video_source); > + > return _exynos_mipi_dsi_get_frame_done_status(dsim); > } > > -int exynos_mipi_dsi_clear_frame_done(struct mipi_dsim_device *dsim) > +int exynos_mipi_dsi_clear_frame_done(struct video_source *video_source) > { > + struct mipi_dsim_device *dsim = container_of(video_source, > + struct mipi_dsim_device, video_source); > _exynos_mipi_dsi_clear_frame_done(dsim); > > return 0; > diff --git a/drivers/video/exynos/exynos_mipi_dsi_common.h b/drivers/video/exynos/exynos_mipi_dsi_common.h > index 4125522..cd89154 100644 > --- a/drivers/video/exynos/exynos_mipi_dsi_common.h > +++ b/drivers/video/exynos/exynos_mipi_dsi_common.h > @@ -18,10 +18,10 @@ > static DECLARE_COMPLETION(dsim_rd_comp); > static DECLARE_COMPLETION(dsim_wr_comp); > > -int exynos_mipi_dsi_wr_data(struct mipi_dsim_device *dsim, unsigned int data_id, > - const unsigned char *data0, unsigned int data_size); > -int exynos_mipi_dsi_rd_data(struct mipi_dsim_device *dsim, unsigned int data_id, > - unsigned int data0, unsigned int req_size, u8 *rx_buf); > +int exynos_mipi_dsi_rd_data(struct video_source *video_source, int data_id, > + u8 data0, u8 *rx_buf,size_t req_size); > +int exynos_mipi_dsi_wr_data(struct video_source *video_source, int data_id, > + u8 *data0, size_t data_size); > irqreturn_t exynos_mipi_dsi_interrupt_handler(int irq, void *dev_id); > void exynos_mipi_dsi_init_interrupt(struct mipi_dsim_device *dsim); > int exynos_mipi_dsi_init_dsim(struct mipi_dsim_device *dsim); > @@ -35,8 +35,8 @@ int exynos_mipi_dsi_set_data_transfer_mode(struct mipi_dsim_device *dsim, > unsigned int mode); > int exynos_mipi_dsi_enable_frame_done_int(struct mipi_dsim_device *dsim, > unsigned int enable); > -int exynos_mipi_dsi_get_frame_done_status(struct mipi_dsim_device *dsim); > -int exynos_mipi_dsi_clear_frame_done(struct mipi_dsim_device *dsim); > +int exynos_mipi_dsi_get_frame_done_status(struct video_source *video_source); > +int exynos_mipi_dsi_clear_frame_done(struct video_source *video_source); > > extern struct fb_info *registered_fb[FB_MAX] __read_mostly; > > diff --git a/include/video/exynos_mipi_dsim.h b/include/video/exynos_mipi_dsim.h > index 83ce5e6..e50438e 100644 > --- a/include/video/exynos_mipi_dsim.h > +++ b/include/video/exynos_mipi_dsim.h > @@ -17,7 +17,7 @@ > > #include <linux/device.h> > #include <linux/fb.h> > - > +#include <video/display.h> > #define PANEL_NAME_SIZE (32) > > /* > @@ -225,9 +225,8 @@ struct mipi_dsim_device { > unsigned int irq; > void __iomem *reg_base; > struct mutex lock; > - > + struct video_source video_source; > struct mipi_dsim_config *dsim_config; > - struct mipi_dsim_master_ops *master_ops; > struct mipi_dsim_lcd_device *dsim_lcd_dev; > struct mipi_dsim_lcd_driver *dsim_lcd_drv; > > -- > 1.7.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- With warm regards, Sachin ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] [RFC] video: exynos mipi dsi: Making Exynos MIPI Complaint with CDF 2013-01-03 5:44 ` Sachin Kamat @ 2013-01-03 8:30 ` Vikas Sajjan 0 siblings, 0 replies; 11+ messages in thread From: Vikas Sajjan @ 2013-01-03 8:30 UTC (permalink / raw) To: Sachin Kamat Cc: joshi, dri-devel, aditya.ps, tomi.valkeinen, laurent.pinchart, Vikas C Sajjan, linux-media Hi Sachin, Thanks for the comments. On Thu, Jan 3, 2013 at 11:14 AM, Sachin Kamat <sachin.kamat@linaro.org> wrote: > Hi Vikas, > > Some nitpicks inline > > Subject: s/Complaint/Compliant > Oops, nice catch. > On 2 January 2013 18:47, Vikas C Sajjan <vikas.sajjan@linaro.org> wrote: >> From: Vikas Sajjan <vikas.sajjan@linaro.org> >> > > Please add some description about this patch here. > sure. >> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org> >> --- >> drivers/video/exynos/exynos_mipi_dsi.c | 46 ++++++++++++++++++------- >> drivers/video/exynos/exynos_mipi_dsi_common.c | 22 ++++++++---- >> drivers/video/exynos/exynos_mipi_dsi_common.h | 12 +++---- >> include/video/exynos_mipi_dsim.h | 5 ++- >> 4 files changed, 56 insertions(+), 29 deletions(-) >> >> diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c >> index 07d70a3..88b2aa9 100644 >> --- a/drivers/video/exynos/exynos_mipi_dsi.c >> +++ b/drivers/video/exynos/exynos_mipi_dsi.c >> @@ -32,14 +32,13 @@ >> #include <linux/notifier.h> >> #include <linux/regulator/consumer.h> >> #include <linux/pm_runtime.h> >> - >> +#include <video/display.h> >> #include <video/exynos_mipi_dsim.h> >> >> #include <plat/fb.h> >> >> #include "exynos_mipi_dsi_common.h" >> #include "exynos_mipi_dsi_lowlevel.h" >> - >> struct mipi_dsim_ddi { >> int bus_id; >> struct list_head list; >> @@ -111,12 +110,13 @@ static void exynos_mipi_update_cfg(struct mipi_dsim_device *dsim) >> exynos_mipi_dsi_stand_by(dsim, 1); >> } >> >> -static int exynos_mipi_dsi_early_blank_mode(struct mipi_dsim_device *dsim, >> +static int exynos_mipi_dsi_early_blank_mode(struct video_source *video_source, >> int power) >> { >> + struct mipi_dsim_device *dsim = container_of(video_source, >> + struct mipi_dsim_device, video_source); >> struct mipi_dsim_lcd_driver *client_drv = dsim->dsim_lcd_drv; >> struct mipi_dsim_lcd_device *client_dev = dsim->dsim_lcd_dev; >> - >> switch (power) { >> case FB_BLANK_POWERDOWN: >> if (dsim->suspended) >> @@ -139,12 +139,13 @@ static int exynos_mipi_dsi_early_blank_mode(struct mipi_dsim_device *dsim, >> return 0; >> } >> >> -static int exynos_mipi_dsi_blank_mode(struct mipi_dsim_device *dsim, int power) >> +static int exynos_mipi_dsi_blank_mode(struct video_source *video_source, int power) >> { >> + struct mipi_dsim_device *dsim = container_of(video_source, >> + struct mipi_dsim_device, video_source); >> struct platform_device *pdev = to_platform_device(dsim->dev); >> struct mipi_dsim_lcd_driver *client_drv = dsim->dsim_lcd_drv; >> struct mipi_dsim_lcd_device *client_dev = dsim->dsim_lcd_dev; >> - >> switch (power) { >> case FB_BLANK_UNBLANK: >> if (!dsim->suspended) >> @@ -319,12 +320,19 @@ static struct mipi_dsim_ddi *exynos_mipi_dsi_bind_lcd_ddi( >> return NULL; >> } >> >> -/* define MIPI-DSI Master operations. */ >> -static struct mipi_dsim_master_ops master_ops = { >> - .cmd_read = exynos_mipi_dsi_rd_data, >> - .cmd_write = exynos_mipi_dsi_wr_data, >> - .get_dsim_frame_done = exynos_mipi_dsi_get_frame_done_status, >> - .clear_dsim_frame_done = exynos_mipi_dsi_clear_frame_done, >> > > +static void panel_dsi_release(struct video_source *src) > > How about exynos_dsi_panel_release in line with other function names? > will modify. >> +{ >> + printk("dsi entity release\n"); > > Please use pr_* function here (instead of printk). > Sure. >> +} >> + >> +static const struct common_video_source_ops dsi_common_ops = { > > Some comments for this place holder would be good. > >> +}; >> + >> +static const struct dsi_video_source_ops exynos_dsi_ops = { >> + .dcs_read = exynos_mipi_dsi_rd_data, >> + .dcs_write = exynos_mipi_dsi_wr_data, >> + .get_frame_done = exynos_mipi_dsi_get_frame_done_status, >> + .clear_frame_done = exynos_mipi_dsi_clear_frame_done, >> .set_early_blank_mode = exynos_mipi_dsi_early_blank_mode, >> .set_blank_mode = exynos_mipi_dsi_blank_mode, >> }; >> @@ -362,7 +370,6 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev) >> } >> >> dsim->dsim_config = dsim_config; >> - dsim->master_ops = &master_ops; >> >> mutex_init(&dsim->lock); >> >> @@ -463,6 +470,19 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev) >> >> dsim->suspended = false; >> >> + dsim->video_source.dev = &pdev->dev; >> + dsim->video_source.release = panel_dsi_release; >> + dsim->video_source.common_ops = &dsi_common_ops; >> + dsim->video_source.ops.dsi = &exynos_dsi_ops; >> + dsim->video_source.name = "exynos"; > > Can't we get this from pdev? > right, will change accordingly. >> + >> + ret = video_source_register(&dsim->video_source); >> + if (ret < 0) { >> + printk("dsi entity register failed\n"); > > Please use pr_* function here (instead of printk). > >> + goto err_bind; >> + } >> + printk("dsi entity registered: %p\n", &dsim->video_source); > > ditto. > > >> + return 0; >> done: >> platform_set_drvdata(pdev, dsim); >> >> diff --git a/drivers/video/exynos/exynos_mipi_dsi_common.c b/drivers/video/exynos/exynos_mipi_dsi_common.c >> index 3cd29a4..e59911e 100644 >> --- a/drivers/video/exynos/exynos_mipi_dsi_common.c >> +++ b/drivers/video/exynos/exynos_mipi_dsi_common.c >> @@ -153,11 +153,12 @@ static void exynos_mipi_dsi_long_data_wr(struct mipi_dsim_device *dsim, >> } >> } >> } >> - >> -int exynos_mipi_dsi_wr_data(struct mipi_dsim_device *dsim, unsigned int data_id, >> - const unsigned char *data0, unsigned int data_size) >> +int exynos_mipi_dsi_wr_data(struct video_source *video_source, int data_id, >> + u8 *data0, size_t data_size) >> { >> unsigned int check_rx_ack = 0; >> + struct mipi_dsim_device *dsim = container_of(video_source, >> + struct mipi_dsim_device, video_source); >> >> if (dsim->state == DSIM_STATE_ULPS) { >> dev_err(dsim->dev, "state is ULPS.\n"); >> @@ -340,12 +341,14 @@ static unsigned int exynos_mipi_dsi_response_size(unsigned int req_size) >> } >> } >> >> -int exynos_mipi_dsi_rd_data(struct mipi_dsim_device *dsim, unsigned int data_id, >> - unsigned int data0, unsigned int req_size, u8 *rx_buf) >> +int exynos_mipi_dsi_rd_data(struct video_source *video_source, int data_id, >> + u8 data0, u8 *rx_buf,size_t req_size) >> { >> unsigned int rx_data, rcv_pkt, i; >> u8 response = 0; >> u16 rxsize; >> + struct mipi_dsim_device *dsim = container_of(video_source, >> + struct mipi_dsim_device, video_source); >> >> if (dsim->state == DSIM_STATE_ULPS) { >> dev_err(dsim->dev, "state is ULPS.\n"); >> @@ -843,13 +846,18 @@ int exynos_mipi_dsi_set_data_transfer_mode(struct mipi_dsim_device *dsim, >> return 0; >> } >> >> -int exynos_mipi_dsi_get_frame_done_status(struct mipi_dsim_device *dsim) >> +int exynos_mipi_dsi_get_frame_done_status(struct video_source *video_source) >> { >> + struct mipi_dsim_device *dsim = container_of(video_source, >> + struct mipi_dsim_device, video_source); >> + >> return _exynos_mipi_dsi_get_frame_done_status(dsim); >> } >> >> -int exynos_mipi_dsi_clear_frame_done(struct mipi_dsim_device *dsim) >> +int exynos_mipi_dsi_clear_frame_done(struct video_source *video_source) >> { >> + struct mipi_dsim_device *dsim = container_of(video_source, >> + struct mipi_dsim_device, video_source); >> _exynos_mipi_dsi_clear_frame_done(dsim); >> >> return 0; >> diff --git a/drivers/video/exynos/exynos_mipi_dsi_common.h b/drivers/video/exynos/exynos_mipi_dsi_common.h >> index 4125522..cd89154 100644 >> --- a/drivers/video/exynos/exynos_mipi_dsi_common.h >> +++ b/drivers/video/exynos/exynos_mipi_dsi_common.h >> @@ -18,10 +18,10 @@ >> static DECLARE_COMPLETION(dsim_rd_comp); >> static DECLARE_COMPLETION(dsim_wr_comp); >> >> -int exynos_mipi_dsi_wr_data(struct mipi_dsim_device *dsim, unsigned int data_id, >> - const unsigned char *data0, unsigned int data_size); >> -int exynos_mipi_dsi_rd_data(struct mipi_dsim_device *dsim, unsigned int data_id, >> - unsigned int data0, unsigned int req_size, u8 *rx_buf); >> +int exynos_mipi_dsi_rd_data(struct video_source *video_source, int data_id, >> + u8 data0, u8 *rx_buf,size_t req_size); >> +int exynos_mipi_dsi_wr_data(struct video_source *video_source, int data_id, >> + u8 *data0, size_t data_size); >> irqreturn_t exynos_mipi_dsi_interrupt_handler(int irq, void *dev_id); >> void exynos_mipi_dsi_init_interrupt(struct mipi_dsim_device *dsim); >> int exynos_mipi_dsi_init_dsim(struct mipi_dsim_device *dsim); >> @@ -35,8 +35,8 @@ int exynos_mipi_dsi_set_data_transfer_mode(struct mipi_dsim_device *dsim, >> unsigned int mode); >> int exynos_mipi_dsi_enable_frame_done_int(struct mipi_dsim_device *dsim, >> unsigned int enable); >> -int exynos_mipi_dsi_get_frame_done_status(struct mipi_dsim_device *dsim); >> -int exynos_mipi_dsi_clear_frame_done(struct mipi_dsim_device *dsim); >> +int exynos_mipi_dsi_get_frame_done_status(struct video_source *video_source); >> +int exynos_mipi_dsi_clear_frame_done(struct video_source *video_source); >> >> extern struct fb_info *registered_fb[FB_MAX] __read_mostly; >> >> diff --git a/include/video/exynos_mipi_dsim.h b/include/video/exynos_mipi_dsim.h >> index 83ce5e6..e50438e 100644 >> --- a/include/video/exynos_mipi_dsim.h >> +++ b/include/video/exynos_mipi_dsim.h >> @@ -17,7 +17,7 @@ >> >> #include <linux/device.h> >> #include <linux/fb.h> >> - >> +#include <video/display.h> >> #define PANEL_NAME_SIZE (32) >> >> /* >> @@ -225,9 +225,8 @@ struct mipi_dsim_device { >> unsigned int irq; >> void __iomem *reg_base; >> struct mutex lock; >> - >> + struct video_source video_source; >> struct mipi_dsim_config *dsim_config; >> - struct mipi_dsim_master_ops *master_ops; >> struct mipi_dsim_lcd_device *dsim_lcd_dev; >> struct mipi_dsim_lcd_driver *dsim_lcd_drv; >> >> -- >> 1.7.9.5 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-media" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > With warm regards, > Sachin > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Regards Vikas Sajjan Samsung Research India (SRI - Bangalore) ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] [RFC] video: display: Adding frame related ops to MIPI DSI video source struct 2013-01-02 13:17 [PATCH 0/2] Making Exynos MIPI Complaint with Common Display Framework Vikas C Sajjan 2013-01-02 13:17 ` [PATCH 1/2] [RFC] video: exynos mipi dsi: Making Exynos MIPI Complaint with CDF Vikas C Sajjan @ 2013-01-02 13:17 ` Vikas C Sajjan 2013-01-03 10:59 ` Tomasz Figa 1 sibling, 1 reply; 11+ messages in thread From: Vikas C Sajjan @ 2013-01-02 13:17 UTC (permalink / raw) To: linux-media, dri-devel Cc: inki.dae, laurent.pinchart, tomi.valkeinen, jesse.barker, aditya.ps From: Vikas Sajjan <vikas.sajjan@linaro.org> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org> --- include/video/display.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/include/video/display.h b/include/video/display.h index b639fd0..fb2f437 100644 --- a/include/video/display.h +++ b/include/video/display.h @@ -117,6 +117,12 @@ struct dsi_video_source_ops { void (*enable_hs)(struct video_source *src, bool enable); + /* frame related */ + int (*get_frame_done)(struct video_source *src); + int (*clear_frame_done)(struct video_source *src); + int (*set_early_blank_mode)(struct video_source *src, int power); + int (*set_blank_mode)(struct video_source *src, int power); + /* data transfer */ int (*dcs_write)(struct video_source *src, int channel, u8 *data, size_t len); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] [RFC] video: display: Adding frame related ops to MIPI DSI video source struct 2013-01-02 13:17 ` [PATCH 2/2] [RFC] video: display: Adding frame related ops to MIPI DSI video source struct Vikas C Sajjan @ 2013-01-03 10:59 ` Tomasz Figa 2013-01-04 4:54 ` Vikas Sajjan 0 siblings, 1 reply; 11+ messages in thread From: Tomasz Figa @ 2013-01-03 10:59 UTC (permalink / raw) To: dri-devel Cc: Vikas C Sajjan, linux-media, tomi.valkeinen, laurent.pinchart, aditya.ps Hi Vikas, On Wednesday 02 of January 2013 18:47:22 Vikas C Sajjan wrote: > From: Vikas Sajjan <vikas.sajjan@linaro.org> > > Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org> > --- > include/video/display.h | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/include/video/display.h b/include/video/display.h > index b639fd0..fb2f437 100644 > --- a/include/video/display.h > +++ b/include/video/display.h > @@ -117,6 +117,12 @@ struct dsi_video_source_ops { > > void (*enable_hs)(struct video_source *src, bool enable); > > + /* frame related */ > + int (*get_frame_done)(struct video_source *src); > + int (*clear_frame_done)(struct video_source *src); > + int (*set_early_blank_mode)(struct video_source *src, int power); > + int (*set_blank_mode)(struct video_source *src, int power); > + I'm not sure if all those extra ops are needed in any way. Looking and Exynos MIPI DSIM driver, set_blank_mode is handling only FB_BLANK_UNBLANK status, which basically equals to the already existing enable operation, while set_early_blank mode handles only FB_BLANK_POWERDOWN, being equal to disable callback. Both get_frame_done and clear_frame_done do not look at anything used at the moment and if frame done status monitoring will be ever needed, I think a better way should be implemented. Best regards, -- Tomasz Figa Samsung Poland R&D Center SW Solution Development, Linux Platform ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] [RFC] video: display: Adding frame related ops to MIPI DSI video source struct 2013-01-03 10:59 ` Tomasz Figa @ 2013-01-04 4:54 ` Vikas Sajjan 2013-01-09 23:35 ` Laurent Pinchart 0 siblings, 1 reply; 11+ messages in thread From: Vikas Sajjan @ 2013-01-04 4:54 UTC (permalink / raw) To: Tomasz Figa Cc: dri-devel, linux-media, inki.dae, tomi.valkeinen, laurent.pinchart, aditya.ps, sunil joshi, Jesse Barker, Rob Clark Hi Mr. Figa, Thanks for reviewing. On 3 January 2013 16:29, Tomasz Figa <t.figa@samsung.com> wrote: > Hi Vikas, > > On Wednesday 02 of January 2013 18:47:22 Vikas C Sajjan wrote: >> From: Vikas Sajjan <vikas.sajjan@linaro.org> >> >> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org> >> --- >> include/video/display.h | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/include/video/display.h b/include/video/display.h >> index b639fd0..fb2f437 100644 >> --- a/include/video/display.h >> +++ b/include/video/display.h >> @@ -117,6 +117,12 @@ struct dsi_video_source_ops { >> >> void (*enable_hs)(struct video_source *src, bool enable); >> >> + /* frame related */ >> + int (*get_frame_done)(struct video_source *src); >> + int (*clear_frame_done)(struct video_source *src); >> + int (*set_early_blank_mode)(struct video_source *src, int power); >> + int (*set_blank_mode)(struct video_source *src, int power); >> + > > I'm not sure if all those extra ops are needed in any way. > > Looking and Exynos MIPI DSIM driver, set_blank_mode is handling only > FB_BLANK_UNBLANK status, which basically equals to the already existing > enable operation, while set_early_blank mode handles only > FB_BLANK_POWERDOWN, being equal to disable callback. > Right, exynos_mipi_dsi_blank_mode() only supports FB_BLANK_UNBLANK as of now, but FB_BLANK_NORMAL will be supported in future. If not for Exynos, i think it will be need for other SoCs which support FB_BLANK_UNBLANK and FB_BLANK_NORMAL. > Both get_frame_done and clear_frame_done do not look at anything used at > the moment and if frame done status monitoring will be ever needed, I > think a better way should be implemented. > You are right, as of now Exynos MIPI DSI Panels are NOT using these callbacks, but as you mentioned we will need frame done status monitoring anyways, so i included these callbacks here. Will check, if we can implement any better method. > Best regards, > -- > Tomasz Figa > Samsung Poland R&D Center > SW Solution Development, Linux Platform > -- Thanks and Regards Vikas Sajjan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] [RFC] video: display: Adding frame related ops to MIPI DSI video source struct 2013-01-04 4:54 ` Vikas Sajjan @ 2013-01-09 23:35 ` Laurent Pinchart 2013-01-10 5:37 ` Vikas Sajjan 2013-01-10 7:43 ` Inki Dae 0 siblings, 2 replies; 11+ messages in thread From: Laurent Pinchart @ 2013-01-09 23:35 UTC (permalink / raw) To: Vikas Sajjan Cc: Tomasz Figa, dri-devel, linux-media, inki.dae, tomi.valkeinen, aditya.ps, sunil joshi, Jesse Barker, Rob Clark Hi Vikas, Thank you for the patch. On Friday 04 January 2013 10:24:04 Vikas Sajjan wrote: > On 3 January 2013 16:29, Tomasz Figa <t.figa@samsung.com> wrote: > > On Wednesday 02 of January 2013 18:47:22 Vikas C Sajjan wrote: > >> From: Vikas Sajjan <vikas.sajjan@linaro.org> > >> > >> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org> > >> --- > >> > >> include/video/display.h | 6 ++++++ > >> 1 file changed, 6 insertions(+) > >> > >> diff --git a/include/video/display.h b/include/video/display.h > >> index b639fd0..fb2f437 100644 > >> --- a/include/video/display.h > >> +++ b/include/video/display.h > >> @@ -117,6 +117,12 @@ struct dsi_video_source_ops { > >> > >> void (*enable_hs)(struct video_source *src, bool enable); > >> > >> + /* frame related */ > >> + int (*get_frame_done)(struct video_source *src); > >> + int (*clear_frame_done)(struct video_source *src); > >> + int (*set_early_blank_mode)(struct video_source *src, int power); > >> + int (*set_blank_mode)(struct video_source *src, int power); > >> + > > > > I'm not sure if all those extra ops are needed in any way. > > > > Looking and Exynos MIPI DSIM driver, set_blank_mode is handling only > > FB_BLANK_UNBLANK status, which basically equals to the already existing > > enable operation, while set_early_blank mode handles only > > FB_BLANK_POWERDOWN, being equal to disable callback. > > Right, exynos_mipi_dsi_blank_mode() only supports FB_BLANK_UNBLANK as > of now, but FB_BLANK_NORMAL will be supported in future. > If not for Exynos, i think it will be need for other SoCs which > support FB_BLANK_UNBLANK and FB_BLANK_NORMAL. Could you please explain in a bit more details what the set_early_blank_mode and set_blank_mode operations do ? > > Both get_frame_done and clear_frame_done do not look at anything used at > > the moment and if frame done status monitoring will be ever needed, I > > think a better way should be implemented. > > You are right, as of now Exynos MIPI DSI Panels are NOT using these > callbacks, but as you mentioned we will need frame done status monitoring > anyways, so i included these callbacks here. Will check, if we can implement > any better method. Do you expect the entity drivers (and in particular the panel drivers) to require frame done notification ? If so, could you explain your use case(s) ? -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] [RFC] video: display: Adding frame related ops to MIPI DSI video source struct 2013-01-09 23:35 ` Laurent Pinchart @ 2013-01-10 5:37 ` Vikas Sajjan 2013-01-10 7:43 ` Inki Dae 1 sibling, 0 replies; 11+ messages in thread From: Vikas Sajjan @ 2013-01-10 5:37 UTC (permalink / raw) To: Laurent Pinchart Cc: Tomasz Figa, dri-devel, linux-media, inki.dae, tomi.valkeinen, aditya.ps, sunil joshi, Jesse Barker, Rob Clark Hi Laurent, On 10 January 2013 05:05, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > Hi Vikas, > > Thank you for the patch. > > On Friday 04 January 2013 10:24:04 Vikas Sajjan wrote: >> On 3 January 2013 16:29, Tomasz Figa <t.figa@samsung.com> wrote: >> > On Wednesday 02 of January 2013 18:47:22 Vikas C Sajjan wrote: >> >> From: Vikas Sajjan <vikas.sajjan@linaro.org> >> >> >> >> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org> >> >> --- >> >> >> >> include/video/display.h | 6 ++++++ >> >> 1 file changed, 6 insertions(+) >> >> >> >> diff --git a/include/video/display.h b/include/video/display.h >> >> index b639fd0..fb2f437 100644 >> >> --- a/include/video/display.h >> >> +++ b/include/video/display.h >> >> @@ -117,6 +117,12 @@ struct dsi_video_source_ops { >> >> >> >> void (*enable_hs)(struct video_source *src, bool enable); >> >> >> >> + /* frame related */ >> >> + int (*get_frame_done)(struct video_source *src); >> >> + int (*clear_frame_done)(struct video_source *src); >> >> + int (*set_early_blank_mode)(struct video_source *src, int power); >> >> + int (*set_blank_mode)(struct video_source *src, int power); >> >> + >> > >> > I'm not sure if all those extra ops are needed in any way. >> > >> > Looking and Exynos MIPI DSIM driver, set_blank_mode is handling only >> > FB_BLANK_UNBLANK status, which basically equals to the already existing >> > enable operation, while set_early_blank mode handles only >> > FB_BLANK_POWERDOWN, being equal to disable callback. >> >> Right, exynos_mipi_dsi_blank_mode() only supports FB_BLANK_UNBLANK as >> of now, but FB_BLANK_NORMAL will be supported in future. >> If not for Exynos, i think it will be need for other SoCs which >> support FB_BLANK_UNBLANK and FB_BLANK_NORMAL. > > Could you please explain in a bit more details what the set_early_blank_mode > and set_blank_mode operations do ? > with reference to Mr. Inki Dae's patch and discussion http://lkml.indiana.edu/hypermail/linux/kernel/1109.1/00413.html http://lkml.indiana.edu/hypermail/linux/kernel/1109.1/02247.html set_early_blank_mode: - sets the framebuffer blank mode. - this callback should be called prior to fb_blank() by a client driver only if needed set_blank_mode: - sets framebuffer blank mode - this callback should be called after fb_blank() by a client driver only if needed. In case of MIPI-DSI based video mode LCD Panel, for lcd power off, the power off commands should be transferred to lcd panel with display and mipi-dsi controller enabled, because the commands is set to lcd panel at vsync porch period, hence set_early_blank_mode() was introduced and should be called prior to fb_blank() as mentioned in the above 2 links. I think Mr. Inki Dae can throw more light on this. >> > Both get_frame_done and clear_frame_done do not look at anything used at >> > the moment and if frame done status monitoring will be ever needed, I >> > think a better way should be implemented. >> >> You are right, as of now Exynos MIPI DSI Panels are NOT using these >> callbacks, but as you mentioned we will need frame done status monitoring >> anyways, so i included these callbacks here. Will check, if we can implement >> any better method. > > Do you expect the entity drivers (and in particular the panel drivers) to > require frame done notification ? If so, could you explain your use case(s) ? > In our Exynos MIPI DSIM H/W, once MIPI DSIM transfers whole image frame, interrupt will raised to indicate the same. as part of the mipi_dsim_master_ops() we have get_dsim_frame_done() and clear_dsim_frame_done() ops. But as of now we are also _NOT_ using these ops in any particular use case. So i guess as of now we can park it, later if we find any need for the same we can add it. > -- > Regards, > > Laurent Pinchart > -- Thanks and Regards Vikas Sajjan SAMSUNG Research India - Banglore. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] [RFC] video: display: Adding frame related ops to MIPI DSI video source struct 2013-01-09 23:35 ` Laurent Pinchart 2013-01-10 5:37 ` Vikas Sajjan @ 2013-01-10 7:43 ` Inki Dae 2013-01-10 11:33 ` Jani Nikula 1 sibling, 1 reply; 11+ messages in thread From: Inki Dae @ 2013-01-10 7:43 UTC (permalink / raw) To: Laurent Pinchart Cc: Vikas Sajjan, sunil joshi, dri-devel, aditya.ps, tomi.valkeinen, Rob Clark, linux-media 2013/1/10 Laurent Pinchart <laurent.pinchart@ideasonboard.com>: > Hi Vikas, > > Thank you for the patch. > > On Friday 04 January 2013 10:24:04 Vikas Sajjan wrote: >> On 3 January 2013 16:29, Tomasz Figa <t.figa@samsung.com> wrote: >> > On Wednesday 02 of January 2013 18:47:22 Vikas C Sajjan wrote: >> >> From: Vikas Sajjan <vikas.sajjan@linaro.org> >> >> >> >> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org> >> >> --- >> >> >> >> include/video/display.h | 6 ++++++ >> >> 1 file changed, 6 insertions(+) >> >> >> >> diff --git a/include/video/display.h b/include/video/display.h >> >> index b639fd0..fb2f437 100644 >> >> --- a/include/video/display.h >> >> +++ b/include/video/display.h >> >> @@ -117,6 +117,12 @@ struct dsi_video_source_ops { >> >> >> >> void (*enable_hs)(struct video_source *src, bool enable); >> >> >> >> + /* frame related */ >> >> + int (*get_frame_done)(struct video_source *src); >> >> + int (*clear_frame_done)(struct video_source *src); >> >> + int (*set_early_blank_mode)(struct video_source *src, int power); >> >> + int (*set_blank_mode)(struct video_source *src, int power); >> >> + >> > >> > I'm not sure if all those extra ops are needed in any way. >> > >> > Looking and Exynos MIPI DSIM driver, set_blank_mode is handling only >> > FB_BLANK_UNBLANK status, which basically equals to the already existing >> > enable operation, while set_early_blank mode handles only >> > FB_BLANK_POWERDOWN, being equal to disable callback. >> >> Right, exynos_mipi_dsi_blank_mode() only supports FB_BLANK_UNBLANK as >> of now, but FB_BLANK_NORMAL will be supported in future. >> If not for Exynos, i think it will be need for other SoCs which >> support FB_BLANK_UNBLANK and FB_BLANK_NORMAL. > > Could you please explain in a bit more details what the set_early_blank_mode > and set_blank_mode operations do ? > >> > Both get_frame_done and clear_frame_done do not look at anything used at >> > the moment and if frame done status monitoring will be ever needed, I >> > think a better way should be implemented. >> >> You are right, as of now Exynos MIPI DSI Panels are NOT using these >> callbacks, but as you mentioned we will need frame done status monitoring >> anyways, so i included these callbacks here. Will check, if we can implement >> any better method. > > Do you expect the entity drivers (and in particular the panel drivers) to > require frame done notification ? If so, could you explain your use case(s) ? > Hi Laurent, As you know, there are two types of MIPI-DSI based lcd panels, RGB and CPU mode. In case of CPU mode lcd panel, it has its own framebuffer internally and the image in the framebuffer is transferred on lcd panel in 60Hz itself. But for this, there is something we should consider. The display controller with CPU mode doens't transfer image data to MIPI-DSI controller itself. So we should set trigger bit of the display controller to 1 to do it and also check whether the data transmission in the framebuffer is done on lcd panel to avoid tearing issue and some confliction issue(A) between read and write operations like below, lcd_panel_frame_done_interrrupt_handler() { ... if (mipi-dsi frame done) trigger display controller; ... } A. the issue that LCD panel can access its own framebuffer while some new data from MIPI-DSI controller is being written in the framebuffer. But I think there might be better way to avoid such thing. Thanks, Inki Dae > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] [RFC] video: display: Adding frame related ops to MIPI DSI video source struct 2013-01-10 7:43 ` Inki Dae @ 2013-01-10 11:33 ` Jani Nikula 0 siblings, 0 replies; 11+ messages in thread From: Jani Nikula @ 2013-01-10 11:33 UTC (permalink / raw) To: Inki Dae, Laurent Pinchart Cc: sunil joshi, dri-devel, Rob Clark, tomi.valkeinen, aditya.ps, Vikas Sajjan, linux-media On Thu, 10 Jan 2013, Inki Dae <inki.dae@samsung.com> wrote: > 2013/1/10 Laurent Pinchart <laurent.pinchart@ideasonboard.com>: >> Hi Vikas, >> >> Thank you for the patch. >> >> On Friday 04 January 2013 10:24:04 Vikas Sajjan wrote: >>> On 3 January 2013 16:29, Tomasz Figa <t.figa@samsung.com> wrote: >>> > On Wednesday 02 of January 2013 18:47:22 Vikas C Sajjan wrote: >>> >> From: Vikas Sajjan <vikas.sajjan@linaro.org> >>> >> >>> >> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org> >>> >> --- >>> >> >>> >> include/video/display.h | 6 ++++++ >>> >> 1 file changed, 6 insertions(+) >>> >> >>> >> diff --git a/include/video/display.h b/include/video/display.h >>> >> index b639fd0..fb2f437 100644 >>> >> --- a/include/video/display.h >>> >> +++ b/include/video/display.h >>> >> @@ -117,6 +117,12 @@ struct dsi_video_source_ops { >>> >> >>> >> void (*enable_hs)(struct video_source *src, bool enable); >>> >> >>> >> + /* frame related */ >>> >> + int (*get_frame_done)(struct video_source *src); >>> >> + int (*clear_frame_done)(struct video_source *src); >>> >> + int (*set_early_blank_mode)(struct video_source *src, int power); >>> >> + int (*set_blank_mode)(struct video_source *src, int power); >>> >> + >>> > >>> > I'm not sure if all those extra ops are needed in any way. >>> > >>> > Looking and Exynos MIPI DSIM driver, set_blank_mode is handling only >>> > FB_BLANK_UNBLANK status, which basically equals to the already existing >>> > enable operation, while set_early_blank mode handles only >>> > FB_BLANK_POWERDOWN, being equal to disable callback. >>> >>> Right, exynos_mipi_dsi_blank_mode() only supports FB_BLANK_UNBLANK as >>> of now, but FB_BLANK_NORMAL will be supported in future. >>> If not for Exynos, i think it will be need for other SoCs which >>> support FB_BLANK_UNBLANK and FB_BLANK_NORMAL. >> >> Could you please explain in a bit more details what the set_early_blank_mode >> and set_blank_mode operations do ? >> >>> > Both get_frame_done and clear_frame_done do not look at anything used at >>> > the moment and if frame done status monitoring will be ever needed, I >>> > think a better way should be implemented. >>> >>> You are right, as of now Exynos MIPI DSI Panels are NOT using these >>> callbacks, but as you mentioned we will need frame done status monitoring >>> anyways, so i included these callbacks here. Will check, if we can implement >>> any better method. >> >> Do you expect the entity drivers (and in particular the panel drivers) to >> require frame done notification ? If so, could you explain your use case(s) ? >> > > Hi Laurent, > > As you know, there are two types of MIPI-DSI based lcd panels, RGB and > CPU mode. In case of CPU mode lcd panel, it has its own framebuffer > internally and the image in the framebuffer is transferred on lcd > panel in 60Hz itself. But for this, there is something we should > consider. The display controller with CPU mode doens't transfer image > data to MIPI-DSI controller itself. So we should set trigger bit of > the display controller to 1 to do it and also check whether the data > transmission in the framebuffer is done on lcd panel to avoid tearing > issue and some confliction issue(A) between read and write operations > like below, Quite right. Just to elaborate, in the MIPI DSI spec the two types are called Video Mode and Command Mode display modules, of which the latter has a framebuffer of its own. Update of the display module framebuffer has to dodge the scanning of the buffer by the display module's controller to avoid tearing. For that, info about the controller's scanline is required. There are basically three ways for this: 1) polling the scanline using DCS get_scan_line command 2) enabling tearing effect reporting, and turning over bus ownership to the display module for subsequent tearing effect signal (vertical blanking) reporting by the module at the specified scanline 3) external GPIO line (outside of DSI PHY spec) to report tearing effect signal For an example, drivers/video/omap2/displays/panel-taal.c supports option #2 via OMAP DSI and option #3 independently. BR, Jani. > > lcd_panel_frame_done_interrrupt_handler() > { > ... > if (mipi-dsi frame done) > trigger display controller; > ... > } > > A. the issue that LCD panel can access its own framebuffer while some > new data from MIPI-DSI controller is being written in the framebuffer. > > But I think there might be better way to avoid such thing. > > Thanks, > Inki Dae > >> -- >> Regards, >> >> Laurent Pinchart >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-01-10 11:33 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-01-02 13:17 [PATCH 0/2] Making Exynos MIPI Complaint with Common Display Framework Vikas C Sajjan 2013-01-02 13:17 ` [PATCH 1/2] [RFC] video: exynos mipi dsi: Making Exynos MIPI Complaint with CDF Vikas C Sajjan 2013-01-03 5:44 ` Sachin Kamat 2013-01-03 8:30 ` Vikas Sajjan 2013-01-02 13:17 ` [PATCH 2/2] [RFC] video: display: Adding frame related ops to MIPI DSI video source struct Vikas C Sajjan 2013-01-03 10:59 ` Tomasz Figa 2013-01-04 4:54 ` Vikas Sajjan 2013-01-09 23:35 ` Laurent Pinchart 2013-01-10 5:37 ` Vikas Sajjan 2013-01-10 7:43 ` Inki Dae 2013-01-10 11:33 ` Jani Nikula
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).