[PATCH v5 2/2] video: backlight: support s6e8ax0 panel driver based on MIPI DSI

[Donghwa Lee]

On Thu, 5, Jan 2012 10:01, Andrew Morton wrote:

> On Wed, 21 Dec 2011 13:25:00 +0900
> Donghwa Lee <dh09.lee at samsung.com> wrote:
> 
>>
>> This patch is amoled panel driver based MIPI DSI interface.
>> S6E8AX0 means it may includes many other ldi controllers, for example,
>> S6E8AA0, S6E8AB0, and so on.
>>
>> This patch can be modified depending on each panel properites. For example,
>> second parameter of panel condition register can be changed depending on
>> ldi controller or amoled type.
>>
>>
>> ...
>>
>> +static unsigned char s6e8ax0_22_gamma_30[] = {
>> +	0xFA, 0x01, 0x60, 0x10, 0x60, 0xF5, 0x00, 0xFF, 0xAD, 0xAF,
>> +	0xBA, 0xC3, 0xD8, 0xC5, 0x9F, 0xC6, 0x9E, 0xC1, 0xDC, 0xC0,
>> +	0x00, 0x61, 0x00, 0x5A, 0x00, 0x74,
>> +};
>> +
>>
>> ...
>>
>> +static unsigned char s6e8ax0_22_gamma_300[] = {
>> +	0xFA, 0x01, 0x60, 0x10, 0x60, 0xB5, 0xD3, 0xBD, 0xB1, 0xD2,
>> +	0xB0, 0xC0, 0xDC, 0xC0, 0x94, 0xBA, 0x91, 0xAC, 0xC5, 0xA9,
>> +	0x00, 0xC2, 0x00, 0xB7, 0x00, 0xED,
>> +};
>> +
>> +static unsigned char *s6e8ax0_22_gamma_table[] = {
>> +	s6e8ax0_22_gamma_30,
>> +	s6e8ax0_22_gamma_50,
>> +	s6e8ax0_22_gamma_60,
>> +	s6e8ax0_22_gamma_70,
>> +	s6e8ax0_22_gamma_80,
>> +	s6e8ax0_22_gamma_90,
>> +	s6e8ax0_22_gamma_100,
>> +	s6e8ax0_22_gamma_120,
>> +	s6e8ax0_22_gamma_130,
>> +	s6e8ax0_22_gamma_140,
>> +	s6e8ax0_22_gamma_150,
>> +	s6e8ax0_22_gamma_160,
>> +	s6e8ax0_22_gamma_170,
>> +	s6e8ax0_22_gamma_180,
>> +	s6e8ax0_22_gamma_190,
>> +	s6e8ax0_22_gamma_200,
>> +	s6e8ax0_22_gamma_210,
>> +	s6e8ax0_22_gamma_220,
>> +	s6e8ax0_22_gamma_230,
>> +	s6e8ax0_22_gamma_240,
>> +	s6e8ax0_22_gamma_250,
>> +	s6e8ax0_22_gamma_260,
>> +	s6e8ax0_22_gamma_270,
>> +	s6e8ax0_22_gamma_280,
>> +	s6e8ax0_22_gamma_300,
>> +};
> 
> I suggest making all the above arrays const.  Otherwise the compiler
> might end up deciding to needlessly allocate space in writeable storage
> for them.
> 
> If that means that ops->cmd_write() needs constification as well then
> let's just do that, for it is the right thing to do.
> 


Ok, I will change it to const arrays with ops->cmd_write() function parameter.

>> +static void s6e8ax0_panel_cond(struct s6e8ax0 *lcd)
>> +{
>> +	struct mipi_dsim_master_ops *ops = lcd_to_master_ops(lcd);
>> +
>> +	unsigned char data_to_send[] = {
>> +		0xf8, 0x3d, 0x35, 0x00, 0x00, 0x00, 0x93, 0x00, 0x3c,
>> +		0x7d, 0x08, 0x27, 0x7d, 0x3f, 0x00, 0x00, 0x00, 0x20,
>> +		0x04, 0x08, 0x6e, 0x00, 0x00, 0x00, 0x02, 0x08, 0x08,
>> +		0x23, 0x23, 0xc0, 0xc8, 0x08, 0x48, 0xc1, 0x00, 0xc1,
>> +		0xff, 0xff, 0xc8
>> +	};
> 
> Arrays like this certainly should be const.  As it stands, the compiler
> needs to generate room on the stack and generate a local copy of the
> array each time this function is called!
> 
>> +	ops->cmd_write(lcd_to_master(lcd), MIPI_DSI_DCS_LONG_WRITE,
>> +		data_to_send, ARRAY_SIZE(data_to_send));
>> +}
>> +
>> +static void s6e8ax0_display_cond(struct s6e8ax0 *lcd)
>> +{
>> +	struct mipi_dsim_master_ops *ops = lcd_to_master_ops(lcd);
>> +	unsigned char data_to_send[] = {
>> +		0xf2, 0x80, 0x03, 0x0d
>> +	};
>> +
>> +	ops->cmd_write(lcd_to_master(lcd), MIPI_DSI_DCS_LONG_WRITE,
>> +		data_to_send, ARRAY_SIZE(data_to_send));
>> +}
>> +
>> +/* Gamma 2.2 Setting (200cd, 7500K, 10MPCD) */
>> +static void s6e8ax0_gamma_cond(struct s6e8ax0 *lcd)
>> +{
>> +	struct mipi_dsim_master_ops *ops = lcd_to_master_ops(lcd);
>> +	unsigned int gamma = lcd->bd->props.brightness;
>> +
>> +	ops->cmd_write(lcd_to_master(lcd), MIPI_DSI_DCS_LONG_WRITE,
>> +			s6e8ax0_22_gamma_table[gamma],
>> +			ARRAY_SIZE(s6e8ax0_22_gamma_table));
> 
> This seems wrong.  ARRAY_SIZE(s6e8ax0_22_gamma_table) does not
> represent the size of s6e8ax0_22_gamma_table[gamma]!
> 
>> +}
>> +
>>
>> ...
>>
>> +static int s6e8ax0_update_gamma_ctrl(struct s6e8ax0 *lcd, int brightness)
>> +{
>> +	struct mipi_dsim_master_ops *ops = lcd_to_master_ops(lcd);
>> +
>> +	ops->cmd_write(lcd_to_master(lcd), MIPI_DSI_DCS_LONG_WRITE,
>> +			s6e8ax0_22_gamma_table[brightness],
>> +			ARRAY_SIZE(s6e8ax0_22_gamma_table));
> 
> Ditto.
> 
>> +	/* update gamma table. */
>> +	s6e8ax0_gamma_update(lcd);
>> +	lcd->gamma = brightness;
>> +
>> +	return 0;
>> +}
>> +
>>
>> ...
>>
>> +static void s6e8ax0_power_on(struct mipi_dsim_lcd_device *dsim_dev, int power)
>> +{
>> +	struct s6e8ax0 *lcd = dev_get_drvdata(&dsim_dev->dev);
>> +
>> +	msleep(lcd->ddi_pd->power_on_delay);
>> +
>> +	/* lcd power on */
>> +	if (power)
>> +		s6e8ax0_regulator_enable(lcd);
>> +	else
>> +		s6e8ax0_regulator_disable(lcd);
>> +
>> +	msleep(lcd->ddi_pd->reset_delay);
>> +
>> +	/* lcd reset */
>> +	if (lcd->ddi_pd->reset)
>> +		lcd->ddi_pd->reset(lcd->ld);
>> +	msleep(5);
>> +}
> 
> Maybe we should hold lcd->lock across this function to prevent other
> code paths from getting in and fiddling with the hardware while it is
> powering on?
> 


In s6e8ax0_regulator_enable()/disable() functions, lcd->lock was used to hold.

>>
>> ...
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>