On Wednesday 18 April 2012 08:28 PM, Tomi Valkeinen wrote:
On Mon, 2012-04-16 at 12:53 +0530, Archit Taneja wrote:
DISPC manager size and DISPC manager blanking parameters(for LCD managers)
follow the shadow register programming model. Currently, they are programmed
directly by the interface drivers.
Make timings(omap_video_timing struct) an overlay_manager_info member, they are
now programmed via the apply mechanism used for programming shadow registers.
The interface driver now call the function dss_mgr_set_timings() which applies
the new timing parameters, rather than directly writing to DISPC registers.
I don't think that works correctly. The omap_overlay_manager_info is
supposed to be set with set_manager_info() by the user of omapdss, to
configure the manager's features. The timings are not supposed to be set
via that mechanism, but with dssdev->set_timings().
This is similar to the info and extra_info for overlay. info has stuff
that omapdss doesn't change, it just uses what the user gives.
extra_info, on the other hand, has omapdss private stuff that the user
does not see. Timings are clearly private stuff in this sense, because
they are set via dssdev->set_timings().
One reason for this is the programming model we use. If the user of
omapdss does get_info() for two overlays, changes the infos, and then
calls set_info() for both overlays and finally apply() for the manager,
we don't do any locking there because omapdss presumes the info is
handled by one user. If, say, the dpi.c would change the info and call
apply at the same time, the configuration could go badly wrong.
I think I get your point. So even though get_info() and set_info() fn's
are spinlock protected, if there are 2 users setting the info, it
doesn't mean that the info they finally written is correct. Is this
example the same thing as what you mean ? :
In order of time:
-user 1 gets an overlay's info
-user 2 gets an overlay's info
-user 1 modifies and sets overlay info
-user 2 sets overlay info without the knowledge of what user 1 did.
So even though we ensure these events happen sequentially, we don't
protect the info across multiple users.
So I think what should be done is to add similar "extra" flags and code
to mgr_priv_data that we have for ovl_priv_data, and add
omap_video_timings to mgr_priv_data (the same way as we have, say,
fifo_low for ovl_priv_data).
And then add similar function to dss_ovl_write_regs_extra() for manager,
and a function like dss_apply_ovl_fifo_thresholds() for timings. And
finally a non-static function to set the timings (used by dpi.c etc),
which calls the similar function to dss_apply_ovl_fifo_thresholds(), and
dss_write_regs() and dss_set_go_bits().
Okay, I'll work on it along these lines.
Tomi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
