Re: [PATCH v3 RESEND] OMAPDSS: DISPC: Improvements to DIGIT sync signal selection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/01/2012 01:28 AM, Tomi Valkeinen wrote:
On Tue, 2012-07-31 at 18:56 -0500, Ricardo Neri wrote:
DSS code wrongly assumes that VENC is always available as source for the external
sync signal for the display controller DIGIT channel. One cannot blindly write/read
the value of DSS_CONTROL[15] as in certain processors (e.g., OMAP5) this operation
may not be valid. If the the sync source is not read correctly, the callers of
dss_get_hdmi_venc_clk_source might make wrong assumptions about, for instance,
video timings.

Logic is added to correctly get the sync signal based on the available displays
in the DIGIT channel. The source is set only if both VENC and HDMI are supported.

Signed-off-by: Ricardo Neri <ricardo.neri@xxxxxx>
---
v3: instead of BUG_ON calls, select only if both VENC and HDMI are available.

I think they do make sense. For example, if somebody calls this function
and tries to set the switch to VENC on an OMAP that doesn't have VENC,
something is wrong. Your new version just skips the register write in
that case, which is ok, but I think we should also yell that something
is wrong.

I'd have the BUG_ONs there, but change them to WARN_ON. I think we
should generally try to have less BUGs and more WARNs. Also, please keep
the comment about the bits being written (VENC_HDMI_SWITCH), it helps to
understand the code.

I will resubmit based on these comments.

  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


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux