Re: [PATCH] drm/panel: simple: Add Innolux N133HSE panel support

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1458 bytes --]

On Wed, Nov 18, 2015 at 10:47:43AM +0100, Marek Vasut wrote:
> From: Sean Cross <xobs@kosagi.com>
> 
> The Innolux N133HSE panel is a 13.3" 1920x1080 panel that contains an
> integrated backlight, and connects via eDP.
> 
> It is used in the Kosagi Novena.
> 
> Signed-off-by: Sean Cross <xobs@kosagi.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> ---
>  drivers/gpu/drm/panel/panel-simple.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)

This is missing a device tree binding document. Otherwise looks good,
except one minor nit below.

> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index f97b73e..d0fd427 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -856,6 +856,29 @@ static const struct panel_desc innolux_n116bge = {
>  	},
>  };
>  
> +static const struct drm_display_mode innolux_n133hse_ea1_mode = {
> +	.clock = 138500,
> +	.hdisplay = 1920,
> +	.hsync_start = 1920 + 46,
> +	.hsync_end = 1920 + 46 + 30,
> +	.htotal = 1920 + 160,

Can you split out the .htotal into its various components, for
consistency with the entries for other panels?

> +	.vdisplay = 1080,
> +	.vsync_start = 1080 + 2,
> +	.vsync_end = 1080 + 2 + 4,
> +	.vtotal = 1080 + 32,

Same for .vtotal here.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel