Hi,

On 07/23/2012 08:26 AM, Manjunathappa, Prakash wrote:
> Hi,
> 
> On Thu, Jul 19, 2012 at 21:07:46, Manjunathappa, Prakash wrote:
>> LCD controller on am335x supports 24bpp raster configuration in addition
>> to ones on da850. LCDC also supports 24bpp in unpacked format having
>> ARGB:8888 32bpp format data in DDR, but it doesn't interpret alpha
>> component of the data.
>>
>> Signed-off-by: Manjunathappa, Prakash <[email protected]>
>> Cc: Anatolij Gustschin <[email protected]>
>> ---
>> Since v2:
>> Fixed additional configurations for 24bpp support.
>> Since v1:
>> Simplified calculation of pseudopalette for FB_VISUAL_TRUECOLOR type.
>>
>>  drivers/video/da8xx-fb.c |  127 
>> ++++++++++++++++++++++++++++++++++------------
>>  1 files changed, 94 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
>> index 47118c7..3d2d0d1 100644
>> --- a/drivers/video/da8xx-fb.c
>> +++ b/drivers/video/da8xx-fb.c
>> @@ -83,6 +83,8 @@
>>  #define LCD_V2_LIDD_CLK_EN          BIT(1)
>>  #define LCD_V2_CORE_CLK_EN          BIT(0)
>>  #define LCD_V2_LPP_B10                      26
>> +#define LCD_V2_TFT_24BPP_MODE               BIT(25)
>> +#define LCD_V2_TFT_24BPP_UNPACK             BIT(26)
>>  
>>  /* LCD Raster Timing 2 Register */
>>  #define LCD_AC_BIAS_TRANSITIONS_PER_INT(x)  ((x) << 16)
>> @@ -153,7 +155,7 @@ struct da8xx_fb_par {
>>      unsigned int            dma_end;
>>      struct clk *lcdc_clk;
>>      int irq;
>> -    unsigned short pseudo_palette[16];
>> +    unsigned long pseudo_palette[16];
> 
> I am still not convinced as sizes of "unsigned long" and "unsigned int" are 
> not
> guaranteed to be 32bit across platforms and compilers, so planning to retain 
> u32.

Yes, if you want something that is always 32 bit you probably should use
u32, that is at least more obvious.
There are a few guarantees in C like
sizeof(short)<=sizeof(int)<=sizeof(long) and short at least being 16
bits and long at least 32 bits that any standard compliant compiler
should honor. And if you limit it to a specific platform/CPU even more
may be assured. But if you want to highlight that you always want to use
32bit u32 is the best choice.

> 
> Florian Tobias Schandinat,
> Can you please comment?

Best regards,

Florian Tobias Schandinat

> 
> Here is the history:
> http://marc.info/?l=linux-fbdev&m=134259216714719&w=2
> 
>>      unsigned int palette_sz;
>>      unsigned int pxl_clk;
>>      int blank;
>> @@ -482,6 +484,9 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par 
>> *par, u32 width, u32 height,
>>  {
>>      u32 reg;
>>  
>> +    if ((bpp > 16) && (lcd_revision == LCD_VERSION_1))
>> +            return -EINVAL;
>> +
>>      /* Set the Panel Width */
>>      /* Pixels per line = (PPL + 1)*16 */
>>      if (lcd_revision == LCD_VERSION_1) {
>> @@ -525,6 +530,12 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par 
>> *par, u32 width, u32 height,
>>      reg = lcdc_read(LCD_RASTER_CTRL_REG) & ~(1 << 8);
>>      if (raster_order)
>>              reg |= LCD_RASTER_ORDER;
>> +
>> +    if (bpp == 24)
>> +            reg |= LCD_V2_TFT_24BPP_MODE;
>> +    else if (bpp == 32)
>> +            reg |= (LCD_V2_TFT_24BPP_MODE | LCD_V2_TFT_24BPP_UNPACK);
>> +
>>      lcdc_write(reg, LCD_RASTER_CTRL_REG);
>>  
>>      switch (bpp) {
>> @@ -532,6 +543,8 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par 
>> *par, u32 width, u32 height,
>>      case 2:
>>      case 4:
>>      case 16:
>> +    case 24:
>> +    case 32:
>>              par->palette_sz = 16 * 2;
>>              break;
>>  
>> @@ -546,6 +559,8 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par 
>> *par, u32 width, u32 height,
>>      return 0;
>>  }
>>  
>> +
>> +#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)
> 
> since multiple FB drivers have re-defined this macro, I will move this to 
> common place(linux/fb.h) and
> convert it as inline function.
> 
> Thanks,
> Prakash
> 
>>  static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
> [...]
> 

_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to