On Tuesday 08 May 2012 06:05 PM, Archit Taneja wrote:
On Tuesday 08 May 2012 05:25 PM, Tomi Valkeinen wrote:
On Tue, 2012-05-08 at 16:52 +0530, Archit Taneja wrote:
On Tuesday 08 May 2012 04:20 PM, Tomi Valkeinen wrote:
Checking the validity of all the settings is a bit tricky, but
currently
I think, as a rule of thumb, we should accept any settings when things
are disabled. So, until the interface driver sets the timings before
enabling the output, all ovl/mgr settings should be allowed. And we
We have 2 ways to go about this, one is to have an initial set of
'always valid' values like I have done, the other option is to ignore
manager timing related checks if the manager is disabled, i.e all
configs are okay. To implement the second option, I think our function
dss_check_settings_low() would get more complicated. We would now have
to pass mp->enabled outside of apply, and pass it to dss_mgr_check()
which would further need to pass this to dss_ovl_check(). Hence I used
the first approach.
I'm not sure about that. We already do it for overlay. In ovl.c we have
dss_ovl_simple_check() and dss_ovl_check(). The simple check sees if the
settings pass basic sanity check. The check sees if all the ovl& mgr
settings are compatible with each other.
Simple check is done when setting the config, called from
dss_ovl_set_info(). The proper check is done later when the actual
config is about to be taken into use.
If mp->enabled == false, can't we just skip dss_check_settings_low()
totally for that manager? We skip the check for ovls the same way.
Okay, this would work better I guess. If someone tries to enable the
manager without setting the timings, that check should fail, and in my
approach, it would have passed, and would have led to bad stuff later.
One change in behaviour with the patch series I see is with the
following use case when connected to Pandaboad's DVI interface:
- Bootup with 1080p resolution
- Try to set 640x480 timings with sysfs.
The older behaviour was that the timings were taken, and a skewed image
was seen(because the overlay size is larger in deimension)
The behaviour after this series is that the mgr_set_timings fails with
the error:
omapdss OVERLAY error: overlay 0 horizontally not inside the display
area (0 + 1920 >= 640)
I guess this is better in a way, a user of DSS is supposed to reallocate
the framebuffer with the desired size and then change the timings.
But with DVI, the problem is that dpi_set_timings already does a ton of
stuff before dss_mgr_set_timings() is called, so we change the dividers
to get the new clock, and then realise that the overlay can't fit
inside, and we are left nowhere. I guess this is a separate problem and
we could tackle it later.
Archit
shouldn't even write any shadow registers until the moment the
output is
enabled.
That's being done correctly even now I guess, with the checks for
mp->enabled in wrtie_regs() and set_go_bits().
Yes, for timings. I was thinking more about the other settings done in
dpi.c currently, like dispc_mgr_set_pol_freq(). That writes directly to
registers, so we need runtime_get for that.
Ok.
Archit
Tomi
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
