On Fri 07 Aug 09:11 PDT 2015, Rob Clark wrote:
> On Fri, Aug 7, 2015 at 9:19 AM, Thierry Reding <thierry.reding@xxxxxxxxx> wrote:
> > On Tue, Jul 21, 2015 at 03:36:02PM -0400, Rob Clark wrote:
[..]
> >> +- compatible: should be "auo,novatek-1080p-vid"
> >
> > This looks a little generic for a compatible string. Can't we get at the
> > specific panel model number that's been used? What if AUO ever produced
> > some other Novatek panel with a 1080p resolution?
>
> Maybe Sony or someone else can chime in? That somewhat generic name
> was all I could get from downstream android kernel. I'm sure there is
> a better possible name, although I have no means to find that out
> myself.
>
We're working on it.
> > Also, what's the -vid suffix for?
>
> the same panel seems to also work in cmd mode.. so idea was to have
> -vid and -cmd compat strings to choose which mode to operate in.
>
An alternative would be to make it a bool property, to indicate video
mode - following how the framework is implemented.
[..]
> >> diff --git a/drivers/gpu/drm/panel/panel-auo-novatek-1080p.c b/drivers/gpu/drm/panel/panel-auo-novatek-1080p.c
> >> +static int auo_panel_init(struct auo_panel *auo)
> >> +{
> >> + struct mipi_dsi_device *dsi = auo->dsi;
> >> + int ret;
> >> +
> >> + dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> >> +
> >> + ret = mipi_dsi_generic_write(dsi, (u8[]){ 0xb0, 0x04 }, 2);
> >
> > I find this notation hard to read. Have you considered moving this into
> > some sort of table that you can loop through? Or perhaps add some
> > helpers, say, mipi_dsi_generic_writeb() and mipi_dsi_dcs_writeb() to
> > help make this more readable?
> >
>
> Yeah, helper macro thing might be a reasonable idea. The table option
> makes it hard to use the helpers for things that are not non-standard,
> or when you need delays, etc..
>
I agree with you here, we don't want lists of data that the driver has
to interpret into writes (of various types) and delays.
>
> >> + if (ret < 0)
> >> + return ret;
> >> +
Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
