Hi Tomi, Archit,
Thanks for your feedback.
On 07/30/2012 06:48 AM, Archit Taneja wrote:
On Monday 30 July 2012 04:41 PM, Tomi Valkeinen wrote:
On Fri, 2012-07-27 at 19:21 -0500, Ricardo Neri wrote:
Hello,
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
rely only on the value of DSS_CONTROL register as certain processors
may not
have VENC (e.g., OMAP5).
These patches add logic to correctly set/get the sync signal based on
the
available displays in the DIGIT channel.
For the HDMI driver, handle the error in the selection of the source.
Is there an actual problem seen that this resolves? If so, It'd be nice
to have a brief description of the problem.
The issue is seen on OMAP5 with a 3.5 kernel. In 3.5 kernel, we had a
hack where we used to divide the TV manager's height by 2 if the
connected interface was VENC. On OMAP5, the bit 15 returns VENC (a zero)
irrespective of what we write.
This won't happen in 3.6 as we have included interlace as a part of the
omap_video_timings struct, and we don't try to check the bit to see
whether we are VENC or HDMI.
I guess we should still put a check to not set the bit on OMAP5 as it
may cause unknown HW behavior, i.e, make the select function like:
void dss_select_hdmi_venc_clk_source()
{
...
displays =
dss_feat_get_supported_displays(OMAP_DSS_CHANNEL_DIGIT);
if ((displays & OMAP_DISPLAY_TYPE_VENC) == 0)
return;
REG_FLD_MOD(DSS_CONTROL, src, 15, 15); /* VENC_HDMI_SWITCH */
}
As Tomi mentions, perhaps this could be complemented with a BUG_ON could
be used if someone tries to use VENC as source when this is not available.
Archit
Does the bit 15 in DSS_CONTROL still exist on OMAP5? What does it return
on OMAP5?
I think it's clear that dss_get_hdmi_venc_clk_source() needs to return a
correct value so that DSS operates correctly. But I'm not sure we need
the extra code in dss_select_hdmi_venc_clk_source().
Yes, the main purpose of these patch was to avoid blindly reading
DSS_CONTROL[15]. The additions to selection were just for completeness.
dss_select_hdmi_venc_clk_source() should be called by venc or hdmi, and
there should be any possibility of using it in the wrong way. For extra
sanity checking there could be a BUG_ON() check in the
dss_select_hdmi_venc_clk_source() to point out if there's a bad bug in
venc or hdmi.
I'd like to keep the lowest level dss-internal functions as simple as
possible, and written in the manner that they presume they are called
correctly. Otherwise we'll end up with lots of new code, checking for
errors that can never happen in practice.
Well, this was an error that happened in practice :).
I will resubmit based on these comments.
Thanks,
Ricardo
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
