Hi Andrzej, Laurent, Thierry.
CCing Steffen Trumtrar,
On 04/29/2014 05:35 PM, YoungJun Cho wrote:
On 04/29/2014 03:02 PM, YoungJun Cho wrote:
Hi Laurent,
Thank you for sharing your idea.
On 04/29/2014 12:05 AM, Laurent Pinchart wrote:
Hi YoungJun,
On Tuesday 22 April 2014 10:24:39 YoungJun Cho wrote:
On 04/22/2014 08:00 AM, Laurent Pinchart wrote:
Hi YoungJun,
Thank you for the patch.
On Monday 21 April 2014 21:28:37 YoungJun Cho wrote:
This patch adds MIPI-DSI command mode based S6E3FA0 AMOLED LCD Panel
driver.
Changelog v2:
- Declares delay, size properties in probe routine instead of DT
Changelog v3:
- Moves CPU timings relevant properties from FIMD DT
(commented by Laurent Pinchart, Andrzej Hajda)
Signed-off-by: YoungJun Cho <yj44.cho@xxxxxxxxxxx>
Acked-by: Inki Dae <inki.dae@xxxxxxxxxxx>
Acked-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
---
drivers/gpu/drm/panel/Kconfig | 7 +
drivers/gpu/drm/panel/Makefile | 1 +
drivers/gpu/drm/panel/panel-s6e3fa0.c | 569
++++++++++++++++++++++++++
3 files changed, 577 insertions(+)
create mode 100644 drivers/gpu/drm/panel/panel-s6e3fa0.c
[snip]
diff --git a/drivers/gpu/drm/panel/panel-s6e3fa0.c
b/drivers/gpu/drm/panel/panel-s6e3fa0.c new file mode 100644
index 0000000..1282678
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-s6e3fa0.c
@@ -0,0 +1,569 @@
[snip]
+static int s6e3fa0_get_modes(struct drm_panel *panel)
+{
+ struct drm_connector *connector = panel->connector;
+ struct s6e3fa0 *ctx = panel_to_s6e3fa0(panel);
+ struct drm_display_mode *mode;
+
+ mode = drm_mode_create(connector->dev);
+ if (!mode) {
+ DRM_ERROR("failed to create a new display mode\n");
+ return 0;
+ }
+
+ drm_display_mode_from_videomode(&ctx->vm, mode);
+ mode->width_mm = ctx->width_mm;
+ mode->height_mm = ctx->height_mm;
+ connector->display_info.width_mm = mode->width_mm;
+ connector->display_info.height_mm = mode->height_mm;
+
+ mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+ mode->private = (void *)&ctx->cpu_timings;
Wouldn't it make sense to create a new get_interface_params (or
similar)
operation for drm_panel to query interface configuration parameters
instead of shoving it in the mode private field ?
You mean "new get_interface_params operation" is different from
get_modes() ?
Till now, struct drm_display_mode and most of mode relevant APIs are
only for video interface.
And CPU interface also needs video mode configurations.
I have a plan to implement the CPU interface relevant APIs like video
mode ones, but I think they should be used under current DRM mode APIs
like fill_modes, get_modes and so on.
So after that implementation, this private field will be replaced by
new ones.
Could you explain it in more detail?
The idea is that the interface parameters (RD/WR signals timings in
this case,
but this could also include MIPI DSI lane configuration or any other
kind of
physical interface parameters) are distinct from the video modes.
Yes. The RD/WR signals timings are distinct from the video modes,
but in my opinion, others are covered by video mode already.
Do you see a need to tie tie interface parameters with
drm_display_mode ? Can
they be mode-specific ? In any case I'd like not to use the private
field of
drm_display_mode. If we need to tie both information together then it
should
be done in a standard way.
I think this cpu-mode-timings is in struct drm_display_mode
(NOT by *private) and requires drm_display_mode_from_cpumode()
because the drm_display_mode_from_videomode() covers only video mode.
I'll try implement it as soon as possible.
For your information, there are two modes in MIPI DSI specification,
which are video mode and command mode.
And CS, RD/WR timings are related with MIPI DBI specification,
VSYNC, HSYNC timings are related with MIPI DPI specification.
So I think all things relevant with command mode should be arranged
the name of command mode(NOT CPU mode, like video mode NOT RGB mode)
in MIPI DSI, and we don't need to consider MIPI DBI like we do MIPI DPI
for video mode.
First, for MIPI command mode, hactive and vactive are only required.
Sorry Andrzej for confusing reply.
I have to modify exynos fimd pixel clock calculating function.
Second, for generic MIPI command mode timing support, commandmode.c and
of_commandmode.c are required in drivers/video.
And the struct drm_panel_command_mode_timings should be moved into
commandmode.h as struct commandmode.
And the last(this is important and I need your ideas), adds this command
mode timings into struct display_timing.
From hierarchical aspects, display_timing(s) should contain command
mode timings too, but it only contains video mode timings.
If I do it, current video mode relevant DTs will be broken.
So IMHO it is required to rename of_*_display_timing() as
of_*_videomode_timing() and add of_*_commandmode_timing() into
of_display_timing.c.
And should add command mode timings into DT like display-timings-command
to distinguish it from display-timings(for videomode).
What do you think about it?
Thank you.
Best regards YJ
Thank you.
Best regards YJ
Thank you,
Best regards YJ.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
