Re: [PATCH 3/7] video: exynos_mipi_dsim: Use the generic PHY driver
Hi Tomasz, On Tue, OCT 28, 2013 21:24, Tomasz Figa wrote: Hi Donghwa, On Monday 28 of October 2013 15:12:08 Donghwa Lee wrote: On Fri, OCT 25, 2013 06:57, Sylwester Nawrocki wrote: On 10/24/2013 05:57 PM, Kishon Vijay Abraham I wrote: On Thursday 24 October 2013 09:12 PM, Sachin Kamat wrote: On 24 October 2013 20:00, Olof Johanssono...@lixom.net wrote: On Wed, Oct 16, 2013 at 9:28 AM, Kishon Vijay Abraham Ikis...@ti.com wrote: diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c index 32e5406..00b3a52 100644 --- a/drivers/video/exynos/exynos_mipi_dsi.c +++ b/drivers/video/exynos/exynos_mipi_dsi.c @@ -156,8 +157,7 @@ static int exynos_mipi_dsi_blank_mode(struct mipi_dsim_device *dsim, int power) exynos_mipi_regulator_enable(dsim); /* enable MIPI-DSI PHY. */ - if (dsim-pd-phy_enable) - dsim-pd-phy_enable(pdev, true); + phy_power_on(dsim-phy); clk_enable(dsim-clock); This introduces the below with exynos_defconfig: ../../drivers/video/exynos/exynos_mipi_dsi.c: In function 'exynos_mipi_dsi_blank_mode': ../../drivers/video/exynos/exynos_mipi_dsi.c:144:26: warning: unused variable 'pdev' [-Wunused-variable] struct platform_device *pdev = to_platform_device(dsim-dev); Sorry about missing that, I only noticed this warning recently and didn't get around to submit a patch. I have already submitted a patch to fix this [1] Thanks a lot guys for fixing this. [1] http://marc.info/?l=linux-fbdevm=138233359617936w=2 Sorry, missed that :-( This MIPI DSIM driver is affectively a dead code in the mainline now, once Exynos become a dt-only platform. I guess it can be deleted for 3.14, once S5P gets converted to the device tree. The new driver using CDF is basically a complete rewrite. Or device tree support should be added to that driver, but I believe it doesn't make sense without CDF. MIPI DSIM driver is not a dead code. There is a steady trickle of patches. It's kind of late, but, I will update it as DT based drivers as soon as possible. And Why do you think that DT support of existing MIPI DSIM is something less than great? First of all, the exynos_mipi_dsim driver has currently no users in mainline kernel, so it is essentially dead code. In addition, on a platform that is the primary candidate for using it, which is Exynos, there is no way to use it, due to no DT support. As I mentioned above, patches are submitted sometimes and I will update this driver as soon as possible to support DT. As for the driver itself, it is not really a great example of good code. It contains a hacks, like calling msleep() without any clear reason and also many coding style issues. I'd prefer to replace it with the new exynos-dsi driver rewritten completely in SRPOL, when CDF is finished. Yes, I know this drivers had been changed about only minor issues and it is not really good code style. And CDF is more good and light. But discussion for CDF is still remaining a kind of requests. If it is merged into linux kernel and many users use it, existing MIPI DSI drivers will be replaced with the new drivers naturally, isn't it? Until that, I and quite a few users will update and code re-factory for this drivers to be more better. BR, Donghwa Lee Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] video: exynos_mipi_dsim: Use the generic PHY driver
On Fri, OCT 25, 2013 06:57, Sylwester Nawrocki wrote: On 10/24/2013 05:57 PM, Kishon Vijay Abraham I wrote: On Thursday 24 October 2013 09:12 PM, Sachin Kamat wrote: On 24 October 2013 20:00, Olof Johanssono...@lixom.net wrote: On Wed, Oct 16, 2013 at 9:28 AM, Kishon Vijay Abraham Ikis...@ti.com wrote: diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c index 32e5406..00b3a52 100644 --- a/drivers/video/exynos/exynos_mipi_dsi.c +++ b/drivers/video/exynos/exynos_mipi_dsi.c @@ -156,8 +157,7 @@ static int exynos_mipi_dsi_blank_mode(struct mipi_dsim_device *dsim, int power) exynos_mipi_regulator_enable(dsim); /* enable MIPI-DSI PHY. */ - if (dsim-pd-phy_enable) - dsim-pd-phy_enable(pdev, true); + phy_power_on(dsim-phy); clk_enable(dsim-clock); This introduces the below with exynos_defconfig: ../../drivers/video/exynos/exynos_mipi_dsi.c: In function 'exynos_mipi_dsi_blank_mode': ../../drivers/video/exynos/exynos_mipi_dsi.c:144:26: warning: unused variable 'pdev' [-Wunused-variable] struct platform_device *pdev = to_platform_device(dsim-dev); Sorry about missing that, I only noticed this warning recently and didn't get around to submit a patch. I have already submitted a patch to fix this [1] Thanks a lot guys for fixing this. [1] http://marc.info/?l=linux-fbdevm=138233359617936w=2 Sorry, missed that :-( This MIPI DSIM driver is affectively a dead code in the mainline now, once Exynos become a dt-only platform. I guess it can be deleted for 3.14, once S5P gets converted to the device tree. The new driver using CDF is basically a complete rewrite. Or device tree support should be added to that driver, but I believe it doesn't make sense without CDF. MIPI DSIM driver is not a dead code. There is a steady trickle of patches. It's kind of late, but, I will update it as DT based drivers as soon as possible. And Why do you think that DT support of existing MIPI DSIM is something less than great? -- Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/5] video: exynos_mipi_dsim: Use the generic PHY driver
On 06/28/2013 00:02, Sylwester Nawrocki wrote:
Use the generic PHY API instead of the platform callback to control
the MIPI DSIM DPHY. The 'phy_label' field is added to the platform
data structure to allow PHY lookup on non-dt platforms.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
Acked-by: Felipe Balbi ba...@ti.com
---
drivers/video/exynos/exynos_mipi_dsi.c | 18 +-
include/video/exynos_mipi_dsim.h |6 --
2 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/video/exynos/exynos_mipi_dsi.c
b/drivers/video/exynos/exynos_mipi_dsi.c
index 32e5406..1f96004 100644
--- a/drivers/video/exynos/exynos_mipi_dsi.c
+++ b/drivers/video/exynos/exynos_mipi_dsi.c
@@ -156,8 +156,7 @@ static int exynos_mipi_dsi_blank_mode(struct
mipi_dsim_device *dsim, int power)
exynos_mipi_regulator_enable(dsim);
/* enable MIPI-DSI PHY. */
- if (dsim-pd-phy_enable)
- dsim-pd-phy_enable(pdev, true);
+ phy_power_on(dsim-phy);
clk_enable(dsim-clock);
@@ -373,6 +372,10 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
return ret;
}
+ dsim-phy = devm_phy_get(pdev-dev, dsim_pd-phy_label);
+ if (IS_ERR(dsim-phy))
+ return PTR_ERR(dsim-phy);
+
dsim-clock = devm_clk_get(pdev-dev, dsim0);
if (IS_ERR(dsim-clock)) {
dev_err(pdev-dev, failed to get dsim clock source\n);
@@ -439,8 +442,7 @@ static int exynos_mipi_dsi_probe(struct platform_device
*pdev)
exynos_mipi_regulator_enable(dsim);
/* enable MIPI-DSI PHY. */
- if (dsim-pd-phy_enable)
- dsim-pd-phy_enable(pdev, true);
+ phy_power_on(dsim-phy);
exynos_mipi_update_cfg(dsim);
@@ -504,9 +506,8 @@ static int exynos_mipi_dsi_suspend(struct device *dev)
if (client_drv client_drv-suspend)
client_drv-suspend(client_dev);
- /* enable MIPI-DSI PHY. */
- if (dsim-pd-phy_enable)
- dsim-pd-phy_enable(pdev, false);
+ /* disable MIPI-DSI PHY. */
+ phy_power_off(dsim-phy);
clk_disable(dsim-clock);
@@ -536,8 +537,7 @@ static int exynos_mipi_dsi_resume(struct device *dev)
exynos_mipi_regulator_enable(dsim);
/* enable MIPI-DSI PHY. */
- if (dsim-pd-phy_enable)
- dsim-pd-phy_enable(pdev, true);
+ phy_power_on(dsim-phy);
clk_enable(dsim-clock);
diff --git a/include/video/exynos_mipi_dsim.h b/include/video/exynos_mipi_dsim.h
index 89dc88a..fd69beb 100644
--- a/include/video/exynos_mipi_dsim.h
+++ b/include/video/exynos_mipi_dsim.h
@@ -216,6 +216,7 @@ struct mipi_dsim_config {
*automatically.
* @e_clk_src: select byte clock source.
* @pd: pointer to MIPI-DSI driver platform data.
+ * @phy: pointer to the generic PHY
*/
struct mipi_dsim_device {
struct device *dev;
@@ -236,6 +237,7 @@ struct mipi_dsim_device {
boolsuspended;
struct mipi_dsim_platform_data *pd;
+ struct phy *phy;
};
/*
@@ -248,7 +250,7 @@ struct mipi_dsim_device {
* @enabled: indicate whether mipi controller got enabled or not.
* @lcd_panel_info: pointer for lcd panel specific structure.
*this structure specifies width, height, timing and polarity and so on.
- * @phy_enable: pointer to a callback controlling D-PHY enable/reset
+ * @phy_label: the generic PHY label
*/
struct mipi_dsim_platform_data {
charlcd_panel_name[PANEL_NAME_SIZE];
@@ -257,7 +259,7 @@ struct mipi_dsim_platform_data {
unsigned intenabled;
void*lcd_panel_info;
- int (*phy_enable)(struct platform_device *pdev, bool on);
+ const char *phy_label;
};
/*
I confirmed that this patch operates well. It looks good to me.
Acked-by: Donghwa Lee dh09@samsung.com
Thank you,
Donghwa Lee
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 3/3] video: exynos: Making s6e8ax0 panel driver compliant with CDF
On Wed, Feb 13, 2013 at 19:01, Vikas Sajjan wrote:
Made necessary changes in s6e8ax0 panel driver as per the CDF-T.
It also removes the dependency on backlight and lcd framework
Signed-off-by: Vikas Sajjanvikas.saj...@linaro.org
---
drivers/video/exynos/s6e8ax0.c | 848 +---
1 file changed, 444 insertions(+), 404 deletions(-)
diff --git a/drivers/video/exynos/s6e8ax0.c b/drivers/video/exynos/s6e8ax0.c
index 7f7b25f..5a17e3c 100644
--- a/drivers/video/exynos/s6e8ax0.c
+++ b/drivers/video/exynos/s6e8ax0.c
@@ -25,6 +25,7 @@
#include linux/backlight.h
#include linux/regulator/consumer.h
+#include video/display.h
#include video/mipi_display.h
#include video/exynos_mipi_dsim.h
@@ -38,8 +39,7 @@
#define POWER_IS_OFF(pwr) ((pwr) == FB_BLANK_POWERDOWN)
#define POWER_IS_NRM(pwr) ((pwr) == FB_BLANK_NORMAL)
-#define lcd_to_master(a) (a-dsim_dev-master)
-#define lcd_to_master_ops(a) ((lcd_to_master(a))-master_ops)
+#define to_panel(p) container_of(p, struct s6e8ax0, entity)
enum {
DSIM_NONE_STATE = 0,
@@ -47,20 +47,34 @@ enum {
DSIM_FRAME_DONE = 2,
};
+/* This structure defines all the properties of a backlight */
+struct backlight_prop {
+ /* Current User requested brightness (0 - max_brightness) */
+ int brightness;
+ /* Maximal value for brightness (read-only) */
+ int max_brightness;
+};
+
+struct panel_platform_data {
+ unsigned intreset_delay;
+ unsigned intpower_on_delay;
+ unsigned intpower_off_delay;
+ const char *video_source_name;
+};
+
struct s6e8ax0 {
- struct device *dev;
- unsigned intpower;
- unsigned intid;
- unsigned intgamma;
- unsigned intacl_enable;
- unsigned intcur_acl;
-
- struct lcd_device *ld;
- struct backlight_device *bd;
-
- struct mipi_dsim_lcd_device *dsim_dev;
- struct lcd_platform_data*ddi_pd;
+ struct platform_device *pdev;
+ struct video_source *src;
+ struct display_entity entity;
+ unsigned intpower;
+ unsigned intid;
+ unsigned intgamma;
+ unsigned intacl_enable;
+ unsigned intcur_acl;
+ boolpanel_reverse;
+ struct lcd_platform_data*plat_data;
struct mutexlock;
+ struct backlight_prop bl_prop;
bool enabled;
};
Could this panel driver use only CDF?
Does not consider the compatibility with backlight and lcd framework?
-static const unsigned char s6e8ax0_22_gamma_30[] = {
+static unsigned char s6e8ax0_22_gamma_30[] = {
0xfa, 0x01, 0x60, 0x10, 0x60, 0xf5, 0x00, 0xff, 0xad, 0xaf,
0xbA, 0xc3, 0xd8, 0xc5, 0x9f, 0xc6, 0x9e, 0xc1, 0xdc, 0xc0,
0x00, 0x61, 0x00, 0x5a, 0x00, 0x74,
};
In all case, you had changed data type to 'static unsigned char'.
Is it need to change all case? Otherwise, for the unity of the code?
Thank you,
Donghwa Lee
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
