On Wed, 2011-11-23 at 15:40 +0530, Archit Taneja wrote: > On Tuesday 22 November 2011 02:51 PM, Tomi Valkeinen wrote: > > The current code uses dsi_video_mode_enable/disable functions to > > enable/disable DISPC output for video mode displays. For command mode > > displays we have no notion in the DISPC side of whether the panel is > > enabled, except when a dss_start_update() call is made. > > > > However, to properly maintain the DISPC state in apply.c, we need to > > know if a manager used for a manual update display is currently in use. > > > > This patch achieves that by changing dsi_video_mode_enable/disable to > > dsi_enable/disable_video_output, which is called by both video and > > command mode displays. For video mode displays it starts the actual > > pixel stream, as it did before. For command mode displays it doesn't do > > anything else than mark that the manager is currently in use. > > dsi_video_mode_enable() doesn't only enable the DISPC output, it also > sends the long packet header to start video mode transfer. > > I think it would be better if we had 2 separate functions, one which > starts/stops DSI video mode, and the other which enables/disables the > DISPC video port. > > This way, a manual update panel would need to call only > dsi_enable/disable_video_output(which just enables or disables the > manager), whereas a video mode panel will need to call both. > > This is just a suggestion though. It's probably okay to have both in the > same function too. We might have to separate them out later if we were > planning to standardise mipi dsi across SoCs. If you think from the panel driver's point of view, it doesn't know about DISPC. It just wants to enable the video stream (on video mode displays). If we had two functions, could only the first be used? I.e. is it possible to just enable the video mode transfer, without enabling DISPC? If not, I'm not sure what would be the use for two separate functions. And even if it can, I'm not sure what use it would be to enable only the video mode output without the actual pixel data from DISPC. It is true that the function in thsi patch is not the best one. For command mode display it's more about reserving the ovl manager for use than actually enabling it. Then again, how I thought the function's purpose was that it enables the DSI video output, but because for command mode there's need to trigger the actual frame transfer later, the function doesn't start the pixel feed for command mode displays. It just "prepares" the output. But even if we had the functions separated, the function called by video mode and command mode displays would be different, as for the former one it enables the pixel stream, and for latter one it just reserves the output. So, I'm fine with changing the function, but the reasoning for what functions we have and what they do should come from the panel driver's perspective, not because of OMAP DSS's HW details. Tomi
Attachment:
signature.asc
Description: This is a digitally signed message part
