| Message ID | 1370503259-16618-1-git-send-email-acourbot@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com> wrote: > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> > --- > Documentation/devicetree/bindings/video/simple-framebuffer.txt | 1 + > drivers/video/simplefb.c | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt > index 3ea4605..70c26f3 100644 > --- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt > +++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt > @@ -12,6 +12,7 @@ Required properties: > - stride: The number of bytes in each line of the framebuffer. > - format: The format of the framebuffer surface. Valid values are: > - r5g6b5 (16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b). > + - a8b8g8r8 (32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r). > > Example: > > diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c > index e2e9e3e..d7041aa 100644 > --- a/drivers/video/simplefb.c > +++ b/drivers/video/simplefb.c > @@ -84,6 +84,7 @@ struct simplefb_format { > > static struct simplefb_format simplefb_formats[] = { > { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} }, > + { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} }, why don't you parse the string? so you will a real generic bindings Best Regards, J. > }; > > struct simplefb_params { > -- > 1.8.3 > -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/06/2013 04:59 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: > > On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com> wrote: > >> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> >> --- >> Documentation/devicetree/bindings/video/simple-framebuffer.txt | 1 + >> drivers/video/simplefb.c | 1 + >> 2 files changed, 2 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt >> index 3ea4605..70c26f3 100644 >> --- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt >> +++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt >> @@ -12,6 +12,7 @@ Required properties: >> - stride: The number of bytes in each line of the framebuffer. >> - format: The format of the framebuffer surface. Valid values are: >> - r5g6b5 (16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b). >> + - a8b8g8r8 (32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r). >> >> Example: >> >> diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c >> index e2e9e3e..d7041aa 100644 >> --- a/drivers/video/simplefb.c >> +++ b/drivers/video/simplefb.c >> @@ -84,6 +84,7 @@ struct simplefb_format { >> >> static struct simplefb_format simplefb_formats[] = { >> { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} }, >> + { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} }, > > why don't you parse the string? > > so you will a real generic bindings Tried that already, got NACKed: https://lkml.org/lkml/2013/5/27/330 The list of modes of this driver should not grow too big. Even in terms of footprint I'd say the list should remain smaller than the parsing code. What we can discuss though is whether we want to keep this a8b8g8r8 syntax or switch to something more standard, say "rgba8888". Alex. -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Jun 6, 2013, at 10:12 AM, Alex Courbot <acourbot@nvidia.com> wrote: > On 06/06/2013 04:59 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: >> >> On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com> wrote: >> >>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> >>> --- >>> Documentation/devicetree/bindings/video/simple-framebuffer.txt | 1 + >>> drivers/video/simplefb.c | 1 + >>> 2 files changed, 2 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt >>> index 3ea4605..70c26f3 100644 >>> --- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt >>> +++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt >>> @@ -12,6 +12,7 @@ Required properties: >>> - stride: The number of bytes in each line of the framebuffer. >>> - format: The format of the framebuffer surface. Valid values are: >>> - r5g6b5 (16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b). >>> + - a8b8g8r8 (32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r). >>> >>> Example: >>> >>> diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c >>> index e2e9e3e..d7041aa 100644 >>> --- a/drivers/video/simplefb.c >>> +++ b/drivers/video/simplefb.c >>> @@ -84,6 +84,7 @@ struct simplefb_format { >>> >>> static struct simplefb_format simplefb_formats[] = { >>> { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} }, >>> + { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} }, >> >> why don't you parse the string? >> >> so you will a real generic bindings > > Tried that already, got NACKed: https://lkml.org/lkml/2013/5/27/330 > > The list of modes of this driver should not grow too big. Even in terms of footprint I'd say the list should remain smaller than the parsing code. > > What we can discuss though is whether we want to keep this a8b8g8r8 syntax or switch to something more standard, say "rgba8888". I'm going to be very honest I do not like the simplefb driver from the beginning but I do found it useful. And as said in it's name it need to be *SIMPLE* Then a huge list of compatible no way otherwise we drop this from the simplefb and make it a generic helper I do not want to see format parser in every drivers this need to handle at video framework level If I see that we start to increase again and again the simplefb I will not accept to merge the code as we must keep it simple Best Regards, J. -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/06/2013 05:24 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: > > On Jun 6, 2013, at 10:12 AM, Alex Courbot <acourbot@nvidia.com> wrote: > >> On 06/06/2013 04:59 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: >>> >>> On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com> wrote: >>> >>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> >>>> --- >>>> Documentation/devicetree/bindings/video/simple-framebuffer.txt | 1 + >>>> drivers/video/simplefb.c | 1 + >>>> 2 files changed, 2 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt >>>> index 3ea4605..70c26f3 100644 >>>> --- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt >>>> +++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt >>>> @@ -12,6 +12,7 @@ Required properties: >>>> - stride: The number of bytes in each line of the framebuffer. >>>> - format: The format of the framebuffer surface. Valid values are: >>>> - r5g6b5 (16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b). >>>> + - a8b8g8r8 (32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r). >>>> >>>> Example: >>>> >>>> diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c >>>> index e2e9e3e..d7041aa 100644 >>>> --- a/drivers/video/simplefb.c >>>> +++ b/drivers/video/simplefb.c >>>> @@ -84,6 +84,7 @@ struct simplefb_format { >>>> >>>> static struct simplefb_format simplefb_formats[] = { >>>> { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} }, >>>> + { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} }, >>> >>> why don't you parse the string? >>> >>> so you will a real generic bindings >> >> Tried that already, got NACKed: https://lkml.org/lkml/2013/5/27/330 >> >> The list of modes of this driver should not grow too big. Even in terms of footprint I'd say the list should remain smaller than the parsing code. >> >> What we can discuss though is whether we want to keep this a8b8g8r8 syntax or switch to something more standard, say "rgba8888". > > I'm going to be very honest I do not like the simplefb driver from the beginning > but I do found it useful. And as said in it's name it need to be *SIMPLE* > > Then a huge list of compatible no way > otherwise we drop this from the simplefb and make it a generic helper > > I do not want to see format parser in every drivers this need to handle at video framework level > > If I see that we start to increase again and again the simplefb I will not accept > to merge the code as we must keep it simple In that case it's probably better to maintain a "simple" list of supported modes, which is what this patch does. Alex. -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 17:27 Thu 06 Jun , Alex Courbot wrote: > On 06/06/2013 05:24 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: > > > >On Jun 6, 2013, at 10:12 AM, Alex Courbot <acourbot@nvidia.com> wrote: > > > >>On 06/06/2013 04:59 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: > >>> > >>>On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com> wrote: > >>> > >>>>Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> > >>>>--- > >>>>Documentation/devicetree/bindings/video/simple-framebuffer.txt | 1 + > >>>>drivers/video/simplefb.c | 1 + > >>>>2 files changed, 2 insertions(+) > >>>> > >>>>diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt > >>>>index 3ea4605..70c26f3 100644 > >>>>--- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt > >>>>+++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt > >>>>@@ -12,6 +12,7 @@ Required properties: > >>>>- stride: The number of bytes in each line of the framebuffer. > >>>>- format: The format of the framebuffer surface. Valid values are: > >>>> - r5g6b5 (16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b). > >>>>+ - a8b8g8r8 (32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r). > >>>> > >>>>Example: > >>>> > >>>>diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c > >>>>index e2e9e3e..d7041aa 100644 > >>>>--- a/drivers/video/simplefb.c > >>>>+++ b/drivers/video/simplefb.c > >>>>@@ -84,6 +84,7 @@ struct simplefb_format { > >>>> > >>>>static struct simplefb_format simplefb_formats[] = { > >>>> { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} }, > >>>>+ { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} }, > >>> > >>>why don't you parse the string? > >>> > >>>so you will a real generic bindings > >> > >>Tried that already, got NACKed: https://lkml.org/lkml/2013/5/27/330 > >> > >>The list of modes of this driver should not grow too big. Even in terms of footprint I'd say the list should remain smaller than the parsing code. > >> > >>What we can discuss though is whether we want to keep this a8b8g8r8 syntax or switch to something more standard, say "rgba8888". > > > >I'm going to be very honest I do not like the simplefb driver from the beginning > >but I do found it useful. And as said in it's name it need to be *SIMPLE* > > > >Then a huge list of compatible no way > >otherwise we drop this from the simplefb and make it a generic helper > > > >I do not want to see format parser in every drivers this need to handle at video framework level > > > >If I see that we start to increase again and again the simplefb I will not accept > >to merge the code as we must keep it simple > > In that case it's probably better to maintain a "simple" list of > supported modes, which is what this patch does. so get out it of the simplefb other drivers can use it Best Regards, J. > Alex. > -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/06/2013 02:12 AM, Alex Courbot wrote: > On 06/06/2013 04:59 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: >> >> On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com> >> wrote: >> >>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> No commit description? It'd be useful to at least justify this by mentioning that some platform will actually use this... ... >>> static struct simplefb_format simplefb_formats[] = { >>> { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} }, >>> + { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} }, >> >> why don't you parse the string? >> >> so you will a real generic bindings > > Tried that already, got NACKed: https://lkml.org/lkml/2013/5/27/330 > > The list of modes of this driver should not grow too big. Even in terms > of footprint I'd say the list should remain smaller than the parsing code. > > What we can discuss though is whether we want to keep this a8b8g8r8 > syntax or switch to something more standard, say "rgba8888". I would prefer to keep the syntax of the new formats consistent, so that if we ever do add format-parsing code rather than table-based lookup, all the existing formats will continue to work unchanged, without any kind of fallback lookup table. -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/06/2013 08:50 AM, Jean-Christophe PLAGNIOL-VILLARD wrote: > On 17:27 Thu 06 Jun , Alex Courbot wrote: >> On 06/06/2013 05:24 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: >>> >>> On Jun 6, 2013, at 10:12 AM, Alex Courbot <acourbot@nvidia.com> wrote: >>> >>>> On 06/06/2013 04:59 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: >>>>> >>>>> On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com> wrote: ... >>>>>> static struct simplefb_format simplefb_formats[] = { >>>>>> { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} }, >>>>>> + { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} }, >>>>> >>>>> why don't you parse the string? Jean-Christophe, I'm afraid I can't tell exactly what you're arguing for. Here, you want code added to parse the string ... This has already been rejected as being over-engineered, and more than this simple driver needs. Even if it were shared code, the only practical use of such a parsing function would be to support this driver, since presumably any other HW-specific driver already knows exactly which format the FB is in, and hence wouldn't need to share this code. >>>>> so you will a real generic bindings >>>> >>>> Tried that already, got NACKed: https://lkml.org/lkml/2013/5/27/330 >>>> >>>> The list of modes of this driver should not grow too big. Even in terms of footprint I'd say the list should remain smaller than the parsing code. >>>> >>>> What we can discuss though is whether we want to keep this a8b8g8r8 syntax or switch to something more standard, say "rgba8888". >>> >>> I'm going to be very honest I do not like the simplefb driver from the beginning >>> but I do found it useful. And as said in it's name it need to be *SIMPLE* >>> >>> Then a huge list of compatible no way >>> otherwise we drop this from the simplefb and make it a generic helper >>> >>> I do not want to see format parser in every drivers this need to handle at video framework level ... yet here you appear to be arguing against using a format parser, or including a format parser ... Note that a lookup table isn't any kind of shared parser; it's just a very tiny and simple table of static data. It seems quite unlikely that this could be a maintenance issue, even if over time a few more entries get added to the table. >>> If I see that we start to increase again and again the simplefb I will not accept >>> to merge the code as we must keep it simple >> >> In that case it's probably better to maintain a "simple" list of >> supported modes, which is what this patch does. > > so get out it of the simplefb other drivers can use it ... yet here you appear to want to move the list of modes into some central location ... I don't think that's useful for the reason I mentioned above: presumably any other HW-specific driver already knows exactly which format the FB is in, and hence wouldn't need to share this code/table. Why don't we simply take this patch to extend this table, and then *if* any other FB driver needs to parse a format from DT, we can move the code(table) to a common location at that time. That will be a trivial change, and one this patch does nothing to make any harder. Making the code/table common before then seems like over-engineering. -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10:11 Thu 06 Jun , Stephen Warren wrote: > On 06/06/2013 02:12 AM, Alex Courbot wrote: > > On 06/06/2013 04:59 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: > >> > >> On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com> > >> wrote: > >> > >>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> > > No commit description? It'd be useful to at least justify this by > mentioning that some platform will actually use this... > > ... > >>> static struct simplefb_format simplefb_formats[] = { > >>> { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} }, > >>> + { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} }, > >> > >> why don't you parse the string? > >> > >> so you will a real generic bindings > > > > Tried that already, got NACKed: https://lkml.org/lkml/2013/5/27/330 > > > > The list of modes of this driver should not grow too big. Even in terms > > of footprint I'd say the list should remain smaller than the parsing code. > > > > What we can discuss though is whether we want to keep this a8b8g8r8 > > syntax or switch to something more standard, say "rgba8888". > > I would prefer to keep the syntax of the new formats consistent, so that > if we ever do add format-parsing code rather than table-based lookup, > all the existing formats will continue to work unchanged, without any > kind of fallback lookup table. I do prefer a format-parsing than any long lookup table that take time at boot time Best Regards, J. -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10:17 Thu 06 Jun , Stephen Warren wrote: > On 06/06/2013 08:50 AM, Jean-Christophe PLAGNIOL-VILLARD wrote: > > On 17:27 Thu 06 Jun , Alex Courbot wrote: > >> On 06/06/2013 05:24 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: > >>> > >>> On Jun 6, 2013, at 10:12 AM, Alex Courbot <acourbot@nvidia.com> wrote: > >>> > >>>> On 06/06/2013 04:59 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: > >>>>> > >>>>> On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com> wrote: > ... > >>>>>> static struct simplefb_format simplefb_formats[] = { > >>>>>> { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} }, > >>>>>> + { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} }, > >>>>> > >>>>> why don't you parse the string? > > Jean-Christophe, I'm afraid I can't tell exactly what you're arguing for. > > Here, you want code added to parse the string ... > > This has already been rejected as being over-engineered, and more than > this simple driver needs. Even if it were shared code, the only > practical use of such a parsing function would be to support this > driver, since presumably any other HW-specific driver already knows > exactly which format the FB is in, and hence wouldn't need to share this > code. > > >>>>> so you will a real generic bindings > >>>> > >>>> Tried that already, got NACKed: https://lkml.org/lkml/2013/5/27/330 > >>>> > >>>> The list of modes of this driver should not grow too big. Even in terms of footprint I'd say the list should remain smaller than the parsing code. > >>>> > >>>> What we can discuss though is whether we want to keep this a8b8g8r8 syntax or switch to something more standard, say "rgba8888". > >>> > >>> I'm going to be very honest I do not like the simplefb driver from the beginning > >>> but I do found it useful. And as said in it's name it need to be *SIMPLE* > >>> > >>> Then a huge list of compatible no way > >>> otherwise we drop this from the simplefb and make it a generic helper > >>> > >>> I do not want to see format parser in every drivers this need to handle at video framework level > > ... yet here you appear to be arguing against using a format parser, or > including a format parser ... > > Note that a lookup table isn't any kind of shared parser; it's just a > very tiny and simple table of static data. It seems quite unlikely that > this could be a maintenance issue, even if over time a few more entries > get added to the table. > > >>> If I see that we start to increase again and again the simplefb I will not accept > >>> to merge the code as we must keep it simple > >> > >> In that case it's probably better to maintain a "simple" list of > >> supported modes, which is what this patch does. > > > > so get out it of the simplefb other drivers can use it > > ... yet here you appear to want to move the list of modes into some > central location ... > > I don't think that's useful for the reason I mentioned above: presumably > any other HW-specific driver already knows exactly which format the FB > is in, and hence wouldn't need to share this code/table. > > Why don't we simply take this patch to extend this table, and then *if* > any other FB driver needs to parse a format from DT, we can move the > code(table) to a common location at that time. That will be a trivial > change, and one this patch does nothing to make any harder. Making the > code/table common before then seems like over-engineering. that why I said *if I see that we start ....* I did reject this patch but put a warning I do not want to see a huge table if so => factorisation or parser Best Regards, J. -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09:33 Thu 06 Jun , Olof Johansson wrote: > On Thu, Jun 6, 2013 at 9:20 AM, Jean-Christophe PLAGNIOL-VILLARD > <plagnioj@jcrosoft.com> wrote: > > On 10:11 Thu 06 Jun , Stephen Warren wrote: > >> On 06/06/2013 02:12 AM, Alex Courbot wrote: > >> > On 06/06/2013 04:59 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: > >> >> > >> >> On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com> > >> >> wrote: > >> >> > >> >>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> > >> > >> No commit description? It'd be useful to at least justify this by > >> mentioning that some platform will actually use this... > >> > >> ... > >> >>> static struct simplefb_format simplefb_formats[] = { > >> >>> { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} }, > >> >>> + { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} }, > >> >> > >> >> why don't you parse the string? > >> >> > >> >> so you will a real generic bindings > >> > > >> > Tried that already, got NACKed: https://lkml.org/lkml/2013/5/27/330 > >> > > >> > The list of modes of this driver should not grow too big. Even in terms > >> > of footprint I'd say the list should remain smaller than the parsing code. > >> > > >> > What we can discuss though is whether we want to keep this a8b8g8r8 > >> > syntax or switch to something more standard, say "rgba8888". > >> > >> I would prefer to keep the syntax of the new formats consistent, so that > >> if we ever do add format-parsing code rather than table-based lookup, > >> all the existing formats will continue to work unchanged, without any > >> kind of fallback lookup table. > > > > I do prefer a format-parsing than any long lookup table that take time at boot > > time > > We're talking about adding a bunch of code instead of one line in a > table. Sorry, I NACK your NACK. If we get more entries we'll add the > parser, but not just for this. as there is no NACK so far I do take as a NACK Best Regards, J. > > If you want to make a framebuffer-subsystem generic and shared helper, > go ahead. I'm sure simplefb will move over to it when it's available. > > Until then: > > Acked-by: Olof Johansson <olof@lixom.net> > > > > -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 6, 2013 at 9:20 AM, Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote: > On 10:11 Thu 06 Jun , Stephen Warren wrote: >> On 06/06/2013 02:12 AM, Alex Courbot wrote: >> > On 06/06/2013 04:59 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: >> >> >> >> On Jun 6, 2013, at 9:20 AM, Alexandre Courbot <acourbot@nvidia.com> >> >> wrote: >> >> >> >>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> >> >> No commit description? It'd be useful to at least justify this by >> mentioning that some platform will actually use this... >> >> ... >> >>> static struct simplefb_format simplefb_formats[] = { >> >>> { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} }, >> >>> + { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} }, >> >> >> >> why don't you parse the string? >> >> >> >> so you will a real generic bindings >> > >> > Tried that already, got NACKed: https://lkml.org/lkml/2013/5/27/330 >> > >> > The list of modes of this driver should not grow too big. Even in terms >> > of footprint I'd say the list should remain smaller than the parsing code. >> > >> > What we can discuss though is whether we want to keep this a8b8g8r8 >> > syntax or switch to something more standard, say "rgba8888". >> >> I would prefer to keep the syntax of the new formats consistent, so that >> if we ever do add format-parsing code rather than table-based lookup, >> all the existing formats will continue to work unchanged, without any >> kind of fallback lookup table. > > I do prefer a format-parsing than any long lookup table that take time at boot > time We're talking about adding a bunch of code instead of one line in a table. Sorry, I NACK your NACK. If we get more entries we'll add the parser, but not just for this. If you want to make a framebuffer-subsystem generic and shared helper, go ahead. I'm sure simplefb will move over to it when it's available. Until then: Acked-by: Olof Johansson <olof@lixom.net> -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/06/2013 10:29 AM, Jean-Christophe PLAGNIOL-VILLARD wrote: ... > I did reject this patch but put a warning I do not want to see a huge table > if so => factorisation or parser Did you mean "accept" here not "reject"? -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/07/2013 01:32 AM, Jean-Christophe PLAGNIOL-VILLARD wrote: >> We're talking about adding a bunch of code instead of one line in a >> table. Sorry, I NACK your NACK. If we get more entries we'll add the >> parser, but not just for this. > as there is no NACK so far I do take as a NACK ... which means? Anyway, I have sent a new version of this patch that includes a summary motivating why we need it. Please explain clearly whether you accept it or not, and if not, what we need to do to add this mode in a satisfactory way. Thanks, Alex. -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt index 3ea4605..70c26f3 100644 --- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt +++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt @@ -12,6 +12,7 @@ Required properties: - stride: The number of bytes in each line of the framebuffer. - format: The format of the framebuffer surface. Valid values are: - r5g6b5 (16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b). + - a8b8g8r8 (32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r). Example: diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c index e2e9e3e..d7041aa 100644 --- a/drivers/video/simplefb.c +++ b/drivers/video/simplefb.c @@ -84,6 +84,7 @@ struct simplefb_format { static struct simplefb_format simplefb_formats[] = { { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} }, + { "a8b8g8r8", 32, {0, 8}, {8, 8}, {16, 8}, {31, 8} }, }; struct simplefb_params {
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> --- Documentation/devicetree/bindings/video/simple-framebuffer.txt | 1 + drivers/video/simplefb.c | 1 + 2 files changed, 2 insertions(+)