Hi Mr. Lee,
thanks for the review.
On 14 February 2013 07:30, Donghwa Lee <dh09.lee@xxxxxxxxxxx> wrote:
> On Wed, Feb 13, 2013 at 19:01, Vikas Sajjan wrote:
>>
>> Made necessary changes in s6e8ax0 panel driver as per the CDF-T.
>> It also removes the dependency on backlight and lcd framework
>>
>> Signed-off-by: Vikas Sajjan<vikas.sajjan@xxxxxxxxxx>
>> ---
>> drivers/video/exynos/s6e8ax0.c | 848
>> +++++++++++++++++++++-------------------
>> 1 file changed, 444 insertions(+), 404 deletions(-)
>>
>> diff --git a/drivers/video/exynos/s6e8ax0.c
>> b/drivers/video/exynos/s6e8ax0.c
>> index 7f7b25f..5a17e3c 100644
>> --- a/drivers/video/exynos/s6e8ax0.c
>> +++ b/drivers/video/exynos/s6e8ax0.c
>> @@ -25,6 +25,7 @@
>> #include <linux/backlight.h>
>> #include <linux/regulator/consumer.h>
>> +#include <video/display.h>
>> #include <video/mipi_display.h>
>> #include <video/exynos_mipi_dsim.h>
>> @@ -38,8 +39,7 @@
>> #define POWER_IS_OFF(pwr) ((pwr) == FB_BLANK_POWERDOWN)
>> #define POWER_IS_NRM(pwr) ((pwr) == FB_BLANK_NORMAL)
>> -#define lcd_to_master(a) (a->dsim_dev->master)
>> -#define lcd_to_master_ops(a) ((lcd_to_master(a))->master_ops)
>> +#define to_panel(p) container_of(p, struct s6e8ax0, entity)
>> enum {
>> DSIM_NONE_STATE = 0,
>> @@ -47,20 +47,34 @@ enum {
>> DSIM_FRAME_DONE = 2,
>> };
>> +/* This structure defines all the properties of a backlight */
>> +struct backlight_prop {
>> + /* Current User requested brightness (0 - max_brightness) */
>> + int brightness;
>> + /* Maximal value for brightness (read-only) */
>> + int max_brightness;
>> +};
>> +
>> +struct panel_platform_data {
>> + unsigned int reset_delay;
>> + unsigned int power_on_delay;
>> + unsigned int power_off_delay;
>> + const char *video_source_name;
>> +};
>> +
>> struct s6e8ax0 {
>> - struct device *dev;
>> - unsigned int power;
>> - unsigned int id;
>> - unsigned int gamma;
>> - unsigned int acl_enable;
>> - unsigned int cur_acl;
>> -
>> - struct lcd_device *ld;
>> - struct backlight_device *bd;
>> -
>> - struct mipi_dsim_lcd_device *dsim_dev;
>> - struct lcd_platform_data *ddi_pd;
>> + struct platform_device *pdev;
>> + struct video_source *src;
>> + struct display_entity entity;
>> + unsigned int power;
>> + unsigned int id;
>> + unsigned int gamma;
>> + unsigned int acl_enable;
>> + unsigned int cur_acl;
>> + bool panel_reverse;
>> + struct lcd_platform_data *plat_data;
>> struct mutex lock;
>> + struct backlight_prop bl_prop;
>> bool enabled;
>> };
>>
>
> Could this panel driver use only CDF?
> Does not consider the compatibility with backlight and lcd framework?
>
as of now CDF does not support backlight and lcd framework functionalities.
Once CDF has the support, we modify the driver to support both CDF and
non CDF way, there by maintaining the backward compatibility with
backlight and lcd framework.
>> -static const unsigned char s6e8ax0_22_gamma_30[] = {
>> +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,
>> };
>
> In all case, you had changed data type to 'static unsigned char'.
> Is it need to change all case? Otherwise, for the unity of the code?
in the CDF-T proposed by Mr. Tomi Valkeinen, the prototype for
"dcs_write" looks as below
int (*dcs_write)(struct video_source *src, int channel, u8 *data, size_t len);
It does not have "const" for the 3rd parameter (u8 *data ), and in our
driver we have all the arrays as "const".
Just to silence the compiler warnings, i had removed the "const" keyword.
>
>
> Thank you,
> Donghwa Lee
>
>
--
Thanks and Regards
Vikas Sajjan
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
