Re: [PATCH v2 09/13] OMAPDSS: SDI: Create a function to set timings

On Tue, Aug 14, 2012 at 11:56 AM, Archit Taneja [off-list ref] wrote:

On Tuesday 14 August 2012 07:14 PM, Tomi Valkeinen wrote:

quoted

On Thu, 2012-08-09 at 17:19 +0530, Archit Taneja wrote:

quoted

Create function omapdss_sdi_set_timings(). Configuring new timings is
done the
same way as before, SDI is disabled, and re-enabled with the new timings
in
dssdev. This just moves the code from the panel drivers to the SDI
driver.

The panel drivers shouldn't be aware of how SDI manages to configure a
new set
of timings. This should be taken care of by the SDI driver itself.

I'm not sure about this one. Although I see that dpi.c does currently
the same thing as you're doing in your patch.

Even HDMI does the same thing.

quoted

One thing is that we should try to remove dssdev uses from the output
drivers, including use of dssdev->state.

Yes, we could do that by keeping a state of the output(and also checking
state of the manager)

quoted

The other thing is that I don't think the output driver should disable &
enable the output during set timings. I think sdi's set_timings should
return EBUSY if the output is enabled. The same way as other
configuration functions should (like dpi_set_data_lines or such).

I'm actually not sure if even the panel driver should disable & enable
the output during set_timings. Perhaps it should be the caller's
(omapdrm or such) responsibility....

My reasoning here is that disabling & enabling the video output is not
invisible to the upper layers, so doing it "in secret" may be bad.

Then again, perhaps timings can be changed freely on some other
platforms, and then it'd be nice if the panel driver wouldn't disable &
enable the output.

So I'm again not quite sure what's the best way to handle this... (of
the dssdev->state I'm sure, its use should be removed from omapdss). Any
thoughts?

I guess it depends on how drm/fb want to use it. I guess an output should
have a set_timings() kind of op if it can do it seamlessly. I guess we can
do that easily in DPI, for example, we could reduce the fps from 60 to 30
without causing an artefacts(I think). For outputs which can't do it, we
could remove the set_timings totally.

fwiw, drm wouldn't try to change timings on the fly..  or at least it
is bracketed by a call to the driver's crtc->prepare() and
crtc->commit() fxns (which in our case disable/enable output).

I haven't seen much userspace that tries to do things like this,
except maybe some apps like xbmc which seem to have some options to
attempt to change timings to align w/ video playback framerate.  I am
a bit curious how many tv's and drivers could support this in a
glitch-free way.

BR,
-R

However, it'll be kind of inconsistent for some outputs to set timings, and
for others to not, and if in the future drm/fb gets exposed to ops too, we
may have dirty checks to see if set_timings is populated or not.

The easiest way would be to make all set_timings just update the copy of the
timings output has, and expect drm/fb to disable and re enable the panel. We
may end up doing unnecessary gpio resets and configuration of the panels
though.

Archit


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html