From: Jani Nikula <jani.nikula@linux.intel.com>
To: Inki Dae <inki.dae@samsung.com>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: sunil joshi <joshi@samsung.com>,
dri-devel@lists.freedesktop.org, Rob Clark <rob.clark@linaro.org>,
tomi.valkeinen@ti.com, aditya.ps@samsung.com,
Vikas Sajjan <vikas.sajjan@linaro.org>,
linux-media@vger.kernel.org
Subject: Re: [PATCH 2/2] [RFC] video: display: Adding frame related ops to MIPI DSI video source struct
Date: Thu, 10 Jan 2013 13:33:11 +0200 [thread overview]
Message-ID: <87ehhtxog8.fsf@intel.com> (raw)
In-Reply-To: <CAAQKjZMsH3xNdTD5D7L90KJhYforgEvL0_6rBKXNyPyB_P04iQ@mail.gmail.com>
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
prev parent reply other threads:[~2013-01-10 11:32 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ehhtxog8.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=aditya.ps@samsung.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=inki.dae@samsung.com \
--cc=joshi@samsung.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=rob.clark@linaro.org \
--cc=tomi.valkeinen@ti.com \
--cc=vikas.sajjan@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox