Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 1 Feb 2021 20:10:01 +0100
From:   Oliver Graute <oliver.graute@gmail.com>
To:     Marco Felsch <m.felsch@pengutronix.de>
Cc:     thierry.reding@gmail.com, Fabio Estevam <festevam@gmail.com>,
        Sam Ravnborg <sam@ravnborg.org>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] drm/panel: simple: add SGD GKTW70SDAD1SD
Message-ID: <20210201191001.GA22816@portage>
References: <1611947364-30688-1-git-send-email-oliver.graute@gmail.com>
 <20210201132736.dbqnyzn7dbu7dsmq@pengutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210201132736.dbqnyzn7dbu7dsmq@pengutronix.de>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk

On 01/02/21, Marco Felsch wrote:
> Hi Oliver,
> 
> thanks for the patch :)
> 
> On 21-01-29 20:09, Oliver Graute wrote:
> > Add support for the Solomon Goldentek Display Model: GKTW70SDAD1SD
> > to panel-simple.
> > 
> > The panel spec from Variscite can be found at:
> > https://www.variscite.com/wp-content/uploads/2017/12/VLCD-CAP-GLD-RGB.pdf
> > 
> > Signed-off-by: Oliver Graute <oliver.graute@gmail.com>
> > Cc: Marco Felsch <m.felsch@pengutronix.de>
> > Cc: Fabio Estevam <festevam@gmail.com>
> > ---
> > 
> > v2:
> > 
> > - changed bpc to 6
> > - set max value of pixelclock
> > - increased hfront_porch and hback_porch
> > - dropped connector-type
> > 
> > adding of bus_format = MEDIA_BUS_FMT_RGB666_1X18 results in wrong colors.
> > omitting bus_format and using some default is good (Tux Pinguin is colored
> > fine)
> > 
> >  drivers/gpu/drm/panel/panel-simple.c | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > index 2be358f..c129a8c 100644
> > --- a/drivers/gpu/drm/panel/panel-simple.c
> > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > @@ -3336,6 +3336,28 @@ static const struct panel_desc satoz_sat050at40h12r2 = {
> >  	.connector_type = DRM_MODE_CONNECTOR_LVDS,
> >  };
> >  
> > +static const struct display_timing sgd_gktw70sdad1sd_timing = {
> > +	.pixelclock = {30000000, 30000000, 40000000},
> > +	.hactive = { 800, 800, 800},
> > +	.hfront_porch = {40, 40, 40},
> > +	.hback_porch = {40, 40, 40},
> > +	.hsync_len = {48, 48, 48},
> > +	.vactive = {480, 480, 480},
> > +	.vfront_porch = {13, 13, 13},
> > +	.vback_porch = {29, 29, 29},
> > +	.vsync_len = {3, 3, 3},
> 
> Please add also:
> 
> 	.flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW |
> 		 DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_PIXDATA_NEGEDGE,
ok will do

> 
> > +};
> > +
> > +static const struct panel_desc sgd_gktw70sdad1sd = {
> > +	.timings = &sgd_gktw70sdad1sd_timing,
> > +	.num_timings = 1,
> > +	.bpc = 6,
> > +	.size = {
> > +		.width = 153,
> > +		.height = 86,
> > +	},
> 
> and:
> 
> 	.delay = {
> 		.prepare = 20 + 20 + 10 + 10, /* T0 + T2 + T3 + T4 */
> 		.enable = 50, /* T5 */
> 		.disable = 50, /* T5 */
> 		.unprepare =  10 + 10 + 20 + 20, /* T4 + T3 + T2 + T0 */
> 	};

ok will do

thx for your review.

Best regards,

Oliver