[PATCH 1/4] drm/tilcdc: add TI LCD Controller DRM driver (v3)

On Wed, Jan 23, 2013 at 7:36 AM, Russell King - ARM Linux
[off-list ref] wrote:

On Wed, Jan 23, 2013 at 07:24:33AM -0600, Rob Clark wrote:

quoted

On Wed, Jan 23, 2013 at 3:42 AM, Jean-Francois Moine [off-list ref] wrote:

quoted

Hi Rob,

As I wanted to re-use your nxp-tda998x driver for the Marvell Dove SoC,
I had a look at your IT LCD driver. Comments below.

Just fyi, you can re-use the nxp-tda998x part independently of tilcdc
(just in case that wasn't clear).

Great, this chip is also used on the cubox too.

cool, it would be great if other platforms could benefit from this as
well.. the out-of-tree nxp driver is just horrid ;-)

The one thing I wonder is how you deal with the VSYNC/HSYNC polarities
that are provided to the TDA9988x chip.  On the cubox, I have to adjust
the mode parameters such that the polarities are fixed up thusly:

        adjusted->flags &= ~(DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NHSYNC |
                             DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC |
                             DRM_MODE_FLAG_PCSYNC | DRM_MODE_FLAG_NCSYNC);

        /* The TDA19988 always requires negative VSYNC? */
        adjusted->flags |= DRM_MODE_FLAG_NVSYNC;

        /* The TDA19988 requires positive HSYNC on 1080p or 720p */
        if ((adjusted->hdisplay == 1920 && adjusted->vdisplay == 1080) ||
            (adjusted->hdisplay == 1280 && adjusted->vdisplay == 720))
                adjusted->flags |= DRM_MODE_FLAG_PHSYNC;
        else
                adjusted->flags |= DRM_MODE_FLAG_NHSYNC;

        return true;

for these resolutions to be displayed correctly.

hmm, I didn't come across this.  Although 1080p seems to be a bit more
than what the little board I'm working on can drive.  I didn't have to
do any special fixup for 720p..

I wonder if you are having to do that with the nxp out of tree driver?
 Maybe it is related to how that driver it is configuring the hw?  It
would be interesting if you hit the same issue w/ the i2c encoder
slave version.

At any rate, if it turns out to be needed, we can add this in
tda998x_encoder_mode_fixup().

Also, when the only output is the HDMI device, reporting the display
"disconnected" and without any modes seems to cause problems - I have to
report "unknown" status when there's nothing connected, and also provide
a default (720p) mode when no EDID is received.

hmm, also did not run into any issues here on my end.  What sort of
problems did you hit?

BR,
-R