[Qemu-devel] [PATCH] hw/display contains files named *_template.h. These are included many times with different values of the DEPTH macro. However, only the DEPTH == 32 case is used. Removed support for DEPTH != 32 in the template headers and in the file that include them.

[Qemu-devel] [PATCH] hw/display contains files named *_template.h. These are included many times with different values of the DEPTH macro. However, only the DEPTH == 32 case is used. Removed support for DEPTH != 32 in the template headers and in the file that include them.

[Nutan Shinde]

Signed-off-by: Nutan Shinde <nutanshinde1992@gmail.com>
---
 hw/display/blizzard.c          |  8 --------
 hw/display/blizzard_template.h | 26 +-------------------------
 hw/display/omap_lcd_template.h |  8 +-------
 hw/display/omap_lcdc.c         |  6 ------
 hw/display/sm501.c             | 17 -----------------
 hw/display/sm501_template.h    |  8 +-------
 6 files changed, 3 insertions(+), 70 deletions(-)

diff --git a/hw/display/blizzard.c b/hw/display/blizzard.c
index c231960..da5e133 100644
--- a/hw/display/blizzard.c
+++ b/hw/display/blizzard.c
@@ -925,14 +925,6 @@ static void blizzard_update_display(void *opaque)
     s->my[1] = 0;
 }
 
-#define DEPTH 8
-#include "blizzard_template.h"
-#define DEPTH 15
-#include "blizzard_template.h"
-#define DEPTH 16
-#include "blizzard_template.h"
-#define DEPTH 24
-#include "blizzard_template.h"
 #define DEPTH 32
 #include "blizzard_template.h"
 
diff --git a/hw/display/blizzard_template.h b/hw/display/blizzard_template.h
index b7ef27c..a4c733d 100644
--- a/hw/display/blizzard_template.h
+++ b/hw/display/blizzard_template.h
@@ -19,31 +19,7 @@
  */
 
 #define SKIP_PIXEL(to)         (to += deststep)
-#if DEPTH == 8
-# define PIXEL_TYPE            uint8_t
-# define COPY_PIXEL(to, from)  do { *to = from; SKIP_PIXEL(to); } while (0)
-# define COPY_PIXEL1(to, from) (*to++ = from)
-#elif DEPTH == 15 || DEPTH == 16
-# define PIXEL_TYPE            uint16_t
-# define COPY_PIXEL(to, from)  do { *to = from; SKIP_PIXEL(to); } while (0)
-# define COPY_PIXEL1(to, from) (*to++ = from)
-#elif DEPTH == 24
-# define PIXEL_TYPE            uint8_t
-# define COPY_PIXEL(to, from) \
-    do {                      \
-        to[0] = from;         \
-        to[1] = (from) >> 8;  \
-        to[2] = (from) >> 16; \
-        SKIP_PIXEL(to);       \
-    } while (0)
-
-# define COPY_PIXEL1(to, from) \
-    do {                       \
-        *to++ = from;          \
-        *to++ = (from) >> 8;   \
-        *to++ = (from) >> 16;  \
-    } while (0)
-#elif DEPTH == 32
+#if DEPTH == 32
 # define PIXEL_TYPE            uint32_t
 # define COPY_PIXEL(to, from)  do { *to = from; SKIP_PIXEL(to); } while (0)
 # define COPY_PIXEL1(to, from) (*to++ = from)
diff --git a/hw/display/omap_lcd_template.h b/hw/display/omap_lcd_template.h
index f0ce71f..c1714ce 100644
--- a/hw/display/omap_lcd_template.h
+++ b/hw/display/omap_lcd_template.h
@@ -27,13 +27,7 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#if DEPTH == 8
-# define BPP 1
-# define PIXEL_TYPE uint8_t
-#elif DEPTH == 15 || DEPTH == 16
-# define BPP 2
-# define PIXEL_TYPE uint16_t
-#elif DEPTH == 32
+#if DEPTH == 32
 # define BPP 4
 # define PIXEL_TYPE uint32_t
 #else
diff --git a/hw/display/omap_lcdc.c b/hw/display/omap_lcdc.c
index ce1058b..84b5f79 100644
--- a/hw/display/omap_lcdc.c
+++ b/hw/display/omap_lcdc.c
@@ -71,12 +71,6 @@ static void omap_lcd_interrupts(struct omap_lcd_panel_s *s)
 
 #define draw_line_func drawfn
 
-#define DEPTH 8
-#include "omap_lcd_template.h"
-#define DEPTH 15
-#include "omap_lcd_template.h"
-#define DEPTH 16
-#include "omap_lcd_template.h"
 #define DEPTH 32
 #include "omap_lcd_template.h"
 
diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 2957243..3fb64b5 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -1170,23 +1170,6 @@ typedef void draw_line_func(uint8_t *d, const uint8_t *s,
 typedef void draw_hwc_line_func(SM501State * s, int crt, uint8_t * palette,
                                 int c_y, uint8_t *d, int width);
 
-#define DEPTH 8
-#include "sm501_template.h"
-
-#define DEPTH 15
-#include "sm501_template.h"
-
-#define BGR_FORMAT
-#define DEPTH 15
-#include "sm501_template.h"
-
-#define DEPTH 16
-#include "sm501_template.h"
-
-#define BGR_FORMAT
-#define DEPTH 16
-#include "sm501_template.h"
-
 #define DEPTH 32
 #include "sm501_template.h"
 
diff --git a/hw/display/sm501_template.h b/hw/display/sm501_template.h
index f33e499..4e5801e 100644
--- a/hw/display/sm501_template.h
+++ b/hw/display/sm501_template.h
@@ -22,13 +22,7 @@
  * THE SOFTWARE.
  */
 
-#if DEPTH == 8
-#define BPP 1
-#define PIXEL_TYPE uint8_t
-#elif DEPTH == 15 || DEPTH == 16
-#define BPP 2
-#define PIXEL_TYPE uint16_t
-#elif DEPTH == 32
+#if DEPTH == 32
 #define BPP 4
 #define PIXEL_TYPE uint32_t
 #else
-- 
2.7.4.windows.1

Re: [Qemu-devel] [PATCH] hw/display contains files named *_template.h. These are included many times with different values of the DEPTH macro. However, only the DEPTH == 32 case is used. Removed support for DEPTH != 32 in the template headers and in the file that include them.

[John Snow]

On 03/22/2016 02:34 AM, Nutan Shinde wrote:
> Signed-off-by: Nutan Shinde <nutanshinde1992@gmail.com>
> ---
>  hw/display/blizzard.c          |  8 --------
>  hw/display/blizzard_template.h | 26 +-------------------------
>  hw/display/omap_lcd_template.h |  8 +-------
>  hw/display/omap_lcdc.c         |  6 ------
>  hw/display/sm501.c             | 17 -----------------
>  hw/display/sm501_template.h    |  8 +-------
>  6 files changed, 3 insertions(+), 70 deletions(-)
> 
> diff --git a/hw/display/blizzard.c b/hw/display/blizzard.c
> index c231960..da5e133 100644
> --- a/hw/display/blizzard.c
> +++ b/hw/display/blizzard.c
> @@ -925,14 +925,6 @@ static void blizzard_update_display(void *opaque)
>      s->my[1] = 0;
>  }
>  
> -#define DEPTH 8
> -#include "blizzard_template.h"
> -#define DEPTH 15
> -#include "blizzard_template.h"
> -#define DEPTH 16
> -#include "blizzard_template.h"
> -#define DEPTH 24
> -#include "blizzard_template.h"
>  #define DEPTH 32
>  #include "blizzard_template.h"
>  
> diff --git a/hw/display/blizzard_template.h b/hw/display/blizzard_template.h
> index b7ef27c..a4c733d 100644
> --- a/hw/display/blizzard_template.h
> +++ b/hw/display/blizzard_template.h
> @@ -19,31 +19,7 @@
>   */
>  
>  #define SKIP_PIXEL(to)         (to += deststep)
> -#if DEPTH == 8
> -# define PIXEL_TYPE            uint8_t
> -# define COPY_PIXEL(to, from)  do { *to = from; SKIP_PIXEL(to); } while (0)
> -# define COPY_PIXEL1(to, from) (*to++ = from)
> -#elif DEPTH == 15 || DEPTH == 16
> -# define PIXEL_TYPE            uint16_t
> -# define COPY_PIXEL(to, from)  do { *to = from; SKIP_PIXEL(to); } while (0)
> -# define COPY_PIXEL1(to, from) (*to++ = from)
> -#elif DEPTH == 24
> -# define PIXEL_TYPE            uint8_t
> -# define COPY_PIXEL(to, from) \
> -    do {                      \
> -        to[0] = from;         \
> -        to[1] = (from) >> 8;  \
> -        to[2] = (from) >> 16; \
> -        SKIP_PIXEL(to);       \
> -    } while (0)
> -
> -# define COPY_PIXEL1(to, from) \
> -    do {                       \
> -        *to++ = from;          \
> -        *to++ = (from) >> 8;   \
> -        *to++ = (from) >> 16;  \
> -    } while (0)
> -#elif DEPTH == 32
> +#if DEPTH == 32
>  # define PIXEL_TYPE            uint32_t
>  # define COPY_PIXEL(to, from)  do { *to = from; SKIP_PIXEL(to); } while (0)
>  # define COPY_PIXEL1(to, from) (*to++ = from)
> diff --git a/hw/display/omap_lcd_template.h b/hw/display/omap_lcd_template.h
> index f0ce71f..c1714ce 100644
> --- a/hw/display/omap_lcd_template.h
> +++ b/hw/display/omap_lcd_template.h
> @@ -27,13 +27,7 @@
>   * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>   */
>  
> -#if DEPTH == 8
> -# define BPP 1
> -# define PIXEL_TYPE uint8_t
> -#elif DEPTH == 15 || DEPTH == 16
> -# define BPP 2
> -# define PIXEL_TYPE uint16_t
> -#elif DEPTH == 32
> +#if DEPTH == 32
>  # define BPP 4
>  # define PIXEL_TYPE uint32_t
>  #else
> diff --git a/hw/display/omap_lcdc.c b/hw/display/omap_lcdc.c
> index ce1058b..84b5f79 100644
> --- a/hw/display/omap_lcdc.c
> +++ b/hw/display/omap_lcdc.c
> @@ -71,12 +71,6 @@ static void omap_lcd_interrupts(struct omap_lcd_panel_s *s)
>  
>  #define draw_line_func drawfn
>  
> -#define DEPTH 8
> -#include "omap_lcd_template.h"
> -#define DEPTH 15
> -#include "omap_lcd_template.h"
> -#define DEPTH 16
> -#include "omap_lcd_template.h"
>  #define DEPTH 32
>  #include "omap_lcd_template.h"
>  
> diff --git a/hw/display/sm501.c b/hw/display/sm501.c
> index 2957243..3fb64b5 100644
> --- a/hw/display/sm501.c
> +++ b/hw/display/sm501.c
> @@ -1170,23 +1170,6 @@ typedef void draw_line_func(uint8_t *d, const uint8_t *s,
>  typedef void draw_hwc_line_func(SM501State * s, int crt, uint8_t * palette,
>                                  int c_y, uint8_t *d, int width);
>  
> -#define DEPTH 8
> -#include "sm501_template.h"
> -
> -#define DEPTH 15
> -#include "sm501_template.h"
> -
> -#define BGR_FORMAT
> -#define DEPTH 15
> -#include "sm501_template.h"
> -
> -#define DEPTH 16
> -#include "sm501_template.h"
> -
> -#define BGR_FORMAT
> -#define DEPTH 16
> -#include "sm501_template.h"
> -
>  #define DEPTH 32
>  #include "sm501_template.h"
>  
> diff --git a/hw/display/sm501_template.h b/hw/display/sm501_template.h
> index f33e499..4e5801e 100644
> --- a/hw/display/sm501_template.h
> +++ b/hw/display/sm501_template.h
> @@ -22,13 +22,7 @@
>   * THE SOFTWARE.
>   */
>  
> -#if DEPTH == 8
> -#define BPP 1
> -#define PIXEL_TYPE uint8_t
> -#elif DEPTH == 15 || DEPTH == 16
> -#define BPP 2
> -#define PIXEL_TYPE uint16_t
> -#elif DEPTH == 32
> +#if DEPTH == 32
>  #define BPP 4
>  #define PIXEL_TYPE uint32_t
>  #else
> 

I can't comment on the patch itself (personally), but it looks like
maybe your patch submission got a little garbled.

The first line of your patch should be the subject line, which should be
limited to somewhere less than about 72 characters or so, following the
standard format of:

"<component>: <effect of patch>"

For example:

"display: remove redundant macro templates" or similar.

Then, the commit message should include your justification:

'hw/display contains files named *_template.h. These are included many
times with different values of the DEPTH macro. However, only the DEPTH
== 32 case is used. Removed support for DEPTH != 32 in the template
headers and in the file that include them.'

Please see:
http://wiki.qemu.org/Contribute/SubmitAPatch#Submitting_your_Patches


I'd recommend using the git format-patch and git send-email tools to
help remove any accidental error from the process.


Next, you need to make sure you CC the relevant maintainers for the
files you are patching, please see:
http://wiki.qemu.org/Contribute/SubmitAPatch#CC_the_relevant_maintainer

In this case ... these files look a little neglected, with no obvious
maintainer. Looks like Peter Maydell is the only person to touch most of
these files in recent times, so maybe CC him.

Thanks,
--js

Re: [Qemu-devel] [PATCH] hw/display contains files named *_template.h. These are included many times with different values of the DEPTH macro. However, only the DEPTH == 32 case is used. Removed support for DEPTH != 32 in the template headers and in the file that include them.

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 944 bytes --]

On 03/22/2016 12:34 AM, Nutan Shinde wrote:
> Signed-off-by: Nutan Shinde <nutanshinde1992@gmail.com>
> ---
>  hw/display/blizzard.c          |  8 --------
>  hw/display/blizzard_template.h | 26 +-------------------------
>  hw/display/omap_lcd_template.h |  8 +-------
>  hw/display/omap_lcdc.c         |  6 ------
>  hw/display/sm501.c             | 17 -----------------
>  hw/display/sm501_template.h    |  8 +-------
>  6 files changed, 3 insertions(+), 70 deletions(-)

How does your patch differ from this earlier posting?

https://lists.gnu.org/archive/html/qemu-devel/2016-03/msg03159.html

I'm going to tweak the Bite Sized Tasks wiki page to mention that
searching the list archives might be in order, since this is not the
first time we've had competing and overlapping patches based on that
task list.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] hw/display contains files named *_template.h. These are included many times with different values of the DEPTH macro. However, only the DEPTH == 32 case is used. Removed support for DEPTH != 32 in the template headers and in the file that include them.

[Nutan Shinde]

[-- Attachment #1: Type: text/plain, Size: 298 bytes --]

Hi John,

My patch is same as the patch mentioned in the link. I just picked up the
task from BitSizedTask page, as it was not crossed.

I guess the issue is resolved then, what should I do in this case? Also,
please tell me, how do I know which tasks are already taken up?

Regards,
Nutan Shinde.

[-- Attachment #2: Type: text/html, Size: 371 bytes --]

Re: [Qemu-devel] [PATCH] hw/display contains files named *_template.h. These are included many times with different values of the DEPTH macro. However, only the DEPTH == 32 case is used. Removed support for DEPTH != 32 in the template headers and in the file that include them.

[John Snow]

On 03/23/2016 01:30 AM, Nutan Shinde wrote:
> Hi John,
> 
> My patch is same as the patch mentioned in the link. I just picked up
> the task from BitSizedTask page, as it was not crossed.
> 
> I guess the issue is resolved then, what should I do in this case? Also,
> please tell me, how do I know which tasks are already taken up?
> 
> Regards,
> Nutan Shinde.
> 

In some cases, since GSoC and Outreachy are approaching, a lot of people
are trying to pick off the Bite Sizes Tasks right now. You may do well
by checking
https://lists.nongnu.org/archive/html/qemu-devel/2016-03/threads.html to
make sure that nobody else has submitted a fix for the task you wish to fix.

In some cases, fixes may have been submitted recently, but aren't
accepted into QEMU just yet. Searching the list archives will help you
determine which is which.

Thanks,
--js

Re: [Qemu-devel] [PATCH] hw/display contains files named *_template.h. These are included many times with different values of the DEPTH macro. However, only the DEPTH == 32 case is used. Removed support for DEPTH != 32 in the template headers and in the file that include them.

[Nutan Shinde]

[-- Attachment #1: Type: text/plain, Size: 1081 bytes --]

Hi,

So, this patch won't be considered, because there is already a patch
related to the same issue, right?


On Wed, Mar 23, 2016 at 7:40 PM, John Snow <jsnow@redhat.com> wrote:

>
>
> On 03/23/2016 01:30 AM, Nutan Shinde wrote:
> > Hi John,
> >
> > My patch is same as the patch mentioned in the link. I just picked up
> > the task from BitSizedTask page, as it was not crossed.
> >
> > I guess the issue is resolved then, what should I do in this case? Also,
> > please tell me, how do I know which tasks are already taken up?
> >
> > Regards,
> > Nutan Shinde.
> >
>
> In some cases, since GSoC and Outreachy are approaching, a lot of people
> are trying to pick off the Bite Sizes Tasks right now. You may do well
> by checking
> https://lists.nongnu.org/archive/html/qemu-devel/2016-03/threads.html to
> make sure that nobody else has submitted a fix for the task you wish to
> fix.
>
> In some cases, fixes may have been submitted recently, but aren't
> accepted into QEMU just yet. Searching the list archives will help you
> determine which is which.
>
> Thanks,
> --js
>

[-- Attachment #2: Type: text/html, Size: 1698 bytes --]

Re: [Qemu-devel] [PATCH] hw/display contains files named *_template.h. These are included many times with different values of the DEPTH macro. However, only the DEPTH == 32 case is used. Removed support for DEPTH != 32 in the template headers and in the file that include them.

[John Snow]

On 03/27/2016 03:33 AM, Nutan Shinde wrote:
> Hi,
> 
> So, this patch won't be considered, because there is already a patch
> related to the same issue, right?
> 

You'd have to follow the other thread to see if the patch was accepted
or not. Even if it isn't, etiquette is generally that it'd be rude to
not allow them a chance to spin their own v2/revision.

So if your patch was sent later than theirs, it's likely this one won't
be considered, correct.

--js

> 
> On Wed, Mar 23, 2016 at 7:40 PM, John Snow <jsnow@redhat.com
> <mailto:jsnow@redhat.com>> wrote:
> 
> 
> 
>     On 03/23/2016 01:30 AM, Nutan Shinde wrote:
>     > Hi John,
>     >
>     > My patch is same as the patch mentioned in the link. I just picked up
>     > the task from BitSizedTask page, as it was not crossed.
>     >
>     > I guess the issue is resolved then, what should I do in this case?
>     Also,
>     > please tell me, how do I know which tasks are already taken up?
>     >
>     > Regards,
>     > Nutan Shinde.
>     >
> 
>     In some cases, since GSoC and Outreachy are approaching, a lot of people
>     are trying to pick off the Bite Sizes Tasks right now. You may do well
>     by checking
>     https://lists.nongnu.org/archive/html/qemu-devel/2016-03/threads.html to
>     make sure that nobody else has submitted a fix for the task you wish
>     to fix.
> 
>     In some cases, fixes may have been submitted recently, but aren't
>     accepted into QEMU just yet. Searching the list archives will help you
>     determine which is which.
> 
>     Thanks,
>     --js
> 
>