diff options
Diffstat (limited to 'results/classifier/deepseek-1/output/changes.')
| -rw-r--r-- | results/classifier/deepseek-1/output/changes./1651167 | 551 | ||||
| -rw-r--r-- | results/classifier/deepseek-1/output/changes./1882851 | 493 |
2 files changed, 0 insertions, 1044 deletions
diff --git a/results/classifier/deepseek-1/output/changes./1651167 b/results/classifier/deepseek-1/output/changes./1651167 deleted file mode 100644 index 33973221..00000000 --- a/results/classifier/deepseek-1/output/changes./1651167 +++ /dev/null @@ -1,551 +0,0 @@ - -hw/ipmi/isa_ipmi_bt.c:283: suspect use of macro ? - -I just had a go at compiling qemu trunk with -llvm trunk. It said: - -hw/ipmi/isa_ipmi_bt.c:283:31: warning: logical not is only applied to the left hand side of this bitwise operator [-Wlogical-not-parentheses] - -Source code is - - IPMI_BT_SET_HBUSY(ib->control_reg, - !IPMI_BT_GET_HBUSY(ib->control_reg)); - -That use of ! causes trouble. The SET and GET -macros are defined as: - -#define IPMI_BT_GET_HBUSY(d) (((d) >> IPMI_BT_HBUSY_BIT) & 0x1) -#define IPMI_BT_SET_HBUSY(d, v) (d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ - (((v & 1) << IPMI_BT_HBUSY_BIT))) - -I can make the compiler shut up by adding extra () in the last -use of v in the SET macro, like this: - -#define IPMI_BT_SET_HBUSY(d, v) (d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ - ((((v) & 1) << IPMI_BT_HBUSY_BIT))) - -I think this is standard good practice when using macro parameters anyway. - -From: Corey Minyard <email address hidden> - -Macro parameters should almost always have () around them when used. -llvm reported an error on this. - -Reported in https://bugs.launchpad.net/bugs/1651167 - -Signed-off-by: Corey Minyard <email address hidden> ---- - hw/ipmi/isa_ipmi_bt.c | 18 +++++++++--------- - 1 file changed, 9 insertions(+), 9 deletions(-) - -diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c -index f036617..8a97314 100644 ---- a/hw/ipmi/isa_ipmi_bt.c -+++ b/hw/ipmi/isa_ipmi_bt.c -@@ -40,37 +40,37 @@ - #define IPMI_BT_CLR_WR_MASK (1 << IPMI_BT_CLR_WR_BIT) - #define IPMI_BT_GET_CLR_WR(d) (((d) >> IPMI_BT_CLR_WR_BIT) & 0x1) - #define IPMI_BT_SET_CLR_WR(d, v) (d) = (((d) & ~IPMI_BT_CLR_WR_MASK) | \ -- (((v & 1) << IPMI_BT_CLR_WR_BIT))) -+ ((((v) & 1) << IPMI_BT_CLR_WR_BIT))) - - #define IPMI_BT_CLR_RD_MASK (1 << IPMI_BT_CLR_RD_BIT) - #define IPMI_BT_GET_CLR_RD(d) (((d) >> IPMI_BT_CLR_RD_BIT) & 0x1) - #define IPMI_BT_SET_CLR_RD(d, v) (d) = (((d) & ~IPMI_BT_CLR_RD_MASK) | \ -- (((v & 1) << IPMI_BT_CLR_RD_BIT))) -+ ((((v) & 1) << IPMI_BT_CLR_RD_BIT))) - - #define IPMI_BT_H2B_ATN_MASK (1 << IPMI_BT_H2B_ATN_BIT) - #define IPMI_BT_GET_H2B_ATN(d) (((d) >> IPMI_BT_H2B_ATN_BIT) & 0x1) - #define IPMI_BT_SET_H2B_ATN(d, v) (d) = (((d) & ~IPMI_BT_H2B_ATN_MASK) | \ -- (((v & 1) << IPMI_BT_H2B_ATN_BIT))) -+ ((((v) & 1) << IPMI_BT_H2B_ATN_BIT))) - - #define IPMI_BT_B2H_ATN_MASK (1 << IPMI_BT_B2H_ATN_BIT) - #define IPMI_BT_GET_B2H_ATN(d) (((d) >> IPMI_BT_B2H_ATN_BIT) & 0x1) - #define IPMI_BT_SET_B2H_ATN(d, v) (d) = (((d) & ~IPMI_BT_B2H_ATN_MASK) | \ -- (((v & 1) << IPMI_BT_B2H_ATN_BIT))) -+ ((((v) & 1) << IPMI_BT_B2H_ATN_BIT))) - - #define IPMI_BT_SMS_ATN_MASK (1 << IPMI_BT_SMS_ATN_BIT) - #define IPMI_BT_GET_SMS_ATN(d) (((d) >> IPMI_BT_SMS_ATN_BIT) & 0x1) - #define IPMI_BT_SET_SMS_ATN(d, v) (d) = (((d) & ~IPMI_BT_SMS_ATN_MASK) | \ -- (((v & 1) << IPMI_BT_SMS_ATN_BIT))) -+ ((((v) & 1) << IPMI_BT_SMS_ATN_BIT))) - - #define IPMI_BT_HBUSY_MASK (1 << IPMI_BT_HBUSY_BIT) - #define IPMI_BT_GET_HBUSY(d) (((d) >> IPMI_BT_HBUSY_BIT) & 0x1) - #define IPMI_BT_SET_HBUSY(d, v) (d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ -- (((v & 1) << IPMI_BT_HBUSY_BIT))) -+ ((((v) & 1) << IPMI_BT_HBUSY_BIT))) - - #define IPMI_BT_BBUSY_MASK (1 << IPMI_BT_BBUSY_BIT) - #define IPMI_BT_GET_BBUSY(d) (((d) >> IPMI_BT_BBUSY_BIT) & 0x1) - #define IPMI_BT_SET_BBUSY(d, v) (d) = (((d) & ~IPMI_BT_BBUSY_MASK) | \ -- (((v & 1) << IPMI_BT_BBUSY_BIT))) -+ ((((v) & 1) << IPMI_BT_BBUSY_BIT))) - - - /* Mask register */ -@@ -80,12 +80,12 @@ - #define IPMI_BT_B2H_IRQ_EN_MASK (1 << IPMI_BT_B2H_IRQ_EN_BIT) - #define IPMI_BT_GET_B2H_IRQ_EN(d) (((d) >> IPMI_BT_B2H_IRQ_EN_BIT) & 0x1) - #define IPMI_BT_SET_B2H_IRQ_EN(d, v) (d) = (((d) & ~IPMI_BT_B2H_IRQ_EN_MASK) | \ -- (((v & 1) << IPMI_BT_B2H_IRQ_EN_BIT))) -+ ((((v) & 1) << IPMI_BT_B2H_IRQ_EN_BIT))) - - #define IPMI_BT_B2H_IRQ_MASK (1 << IPMI_BT_B2H_IRQ_BIT) - #define IPMI_BT_GET_B2H_IRQ(d) (((d) >> IPMI_BT_B2H_IRQ_BIT) & 0x1) - #define IPMI_BT_SET_B2H_IRQ(d, v) (d) = (((d) & ~IPMI_BT_B2H_IRQ_MASK) | \ -- (((v & 1) << IPMI_BT_B2H_IRQ_BIT))) -+ ((((v) & 1) << IPMI_BT_B2H_IRQ_BIT))) - - typedef struct IPMIBT { - IPMIBmc *bmc; --- -2.7.4 - - - -On 12/22/2016 08:30 AM, <email address hidden> wrote: -> From: Corey Minyard <email address hidden> -> -> Macro parameters should almost always have () around them when used. -> llvm reported an error on this. -> -> Reported in https://bugs.launchpad.net/bugs/1651167 -> -> Signed-off-by: Corey Minyard <email address hidden> -> --- -> hw/ipmi/isa_ipmi_bt.c | 18 +++++++++--------- -> 1 file changed, 9 insertions(+), 9 deletions(-) -> -> diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c -> index f036617..8a97314 100644 -> --- a/hw/ipmi/isa_ipmi_bt.c -> +++ b/hw/ipmi/isa_ipmi_bt.c -> @@ -40,37 +40,37 @@ -> #define IPMI_BT_CLR_WR_MASK (1 << IPMI_BT_CLR_WR_BIT) -> #define IPMI_BT_GET_CLR_WR(d) (((d) >> IPMI_BT_CLR_WR_BIT) & 0x1) -> #define IPMI_BT_SET_CLR_WR(d, v) (d) = (((d) & ~IPMI_BT_CLR_WR_MASK) | \ - -Still under-parenthesized, if the result of IPMI_BT_SET_CLR_WR() is used -in any larger expression; - -> - (((v & 1) << IPMI_BT_CLR_WR_BIT))) -> + ((((v) & 1) << IPMI_BT_CLR_WR_BIT))) - -and at the same time, you (still) have a redundant set on the second line. - -Better would be: - -((d) = (((d) & ~IPMI_BD_CLR_WR_MASK) | \ - (((v) & 1) << IPMI_BT_CLR_WR_BIT))) - -> -> #define IPMI_BT_CLR_RD_MASK (1 << IPMI_BT_CLR_RD_BIT) -> #define IPMI_BT_GET_CLR_RD(d) (((d) >> IPMI_BT_CLR_RD_BIT) & 0x1) -> #define IPMI_BT_SET_CLR_RD(d, v) (d) = (((d) & ~IPMI_BT_CLR_RD_MASK) | \ -> - (((v & 1) << IPMI_BT_CLR_RD_BIT))) -> + ((((v) & 1) << IPMI_BT_CLR_RD_BIT))) - -and again, throughout the patch. - - --- -Eric Blake eblake redhat com +1-919-301-3266 -Libvirt virtualization library http://libvirt.org - - - -On 12/22/2016 09:01 AM, Eric Blake wrote: -> On 12/22/2016 08:30 AM, <email address hidden> wrote: ->> From: Corey Minyard <email address hidden> ->> ->> Macro parameters should almost always have () around them when used. ->> llvm reported an error on this. ->> ->> Reported in https://bugs.launchpad.net/bugs/1651167 ->> ->> Signed-off-by: Corey Minyard <email address hidden> ->> --- ->> hw/ipmi/isa_ipmi_bt.c | 18 +++++++++--------- ->> 1 file changed, 9 insertions(+), 9 deletions(-) ->> ->> diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c ->> index f036617..8a97314 100644 ->> --- a/hw/ipmi/isa_ipmi_bt.c ->> +++ b/hw/ipmi/isa_ipmi_bt.c ->> @@ -40,37 +40,37 @@ ->> #define IPMI_BT_CLR_WR_MASK (1 << IPMI_BT_CLR_WR_BIT) ->> #define IPMI_BT_GET_CLR_WR(d) (((d) >> IPMI_BT_CLR_WR_BIT) & 0x1) ->> #define IPMI_BT_SET_CLR_WR(d, v) (d) = (((d) & ~IPMI_BT_CLR_WR_MASK) | \ -> Still under-parenthesized, if the result of IPMI_BT_SET_CLR_WR() is used -> in any larger expression; - -I wasn't thinking about this being used in a larger expression, but it -should be protected, -I suppose. I'll re-submit with that fixed and the extra () removed. - -Thanks, - --corey - ->> - (((v & 1) << IPMI_BT_CLR_WR_BIT))) ->> + ((((v) & 1) << IPMI_BT_CLR_WR_BIT))) -> and at the same time, you (still) have a redundant set on the second line. -> -> Better would be: -> -> ((d) = (((d) & ~IPMI_BD_CLR_WR_MASK) | \ -> (((v) & 1) << IPMI_BT_CLR_WR_BIT))) -> ->> ->> #define IPMI_BT_CLR_RD_MASK (1 << IPMI_BT_CLR_RD_BIT) ->> #define IPMI_BT_GET_CLR_RD(d) (((d) >> IPMI_BT_CLR_RD_BIT) & 0x1) ->> #define IPMI_BT_SET_CLR_RD(d, v) (d) = (((d) & ~IPMI_BT_CLR_RD_MASK) | \ ->> - (((v & 1) << IPMI_BT_CLR_RD_BIT))) ->> + ((((v) & 1) << IPMI_BT_CLR_RD_BIT))) -> and again, throughout the patch. -> -> - - - -From: Corey Minyard <email address hidden> - -Macro parameters should almost always have () around them when used. -llvm reported an error on this. - -Remove redundant parenthesis and put parenthesis around the entire -macros with assignments in case they are used in an expression. - -Remove some unused macros. - -Reported in https://bugs.launchpad.net/bugs/1651167 - -Signed-off-by: Corey Minyard <email address hidden> ---- - hw/ipmi/isa_ipmi_bt.c | 34 ++++++++++++---------------------- - 1 file changed, 12 insertions(+), 22 deletions(-) - -Changes in v2: - * Put parenthesis around macros that had assignment in them. - * Removed some redundant parenthesis. - * Removed some macros that were not used. - -diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c -index f036617..68bf5cd 100644 ---- a/hw/ipmi/isa_ipmi_bt.c -+++ b/hw/ipmi/isa_ipmi_bt.c -@@ -37,40 +37,30 @@ - #define IPMI_BT_HBUSY_BIT 6 - #define IPMI_BT_BBUSY_BIT 7 - --#define IPMI_BT_CLR_WR_MASK (1 << IPMI_BT_CLR_WR_BIT) - #define IPMI_BT_GET_CLR_WR(d) (((d) >> IPMI_BT_CLR_WR_BIT) & 0x1) --#define IPMI_BT_SET_CLR_WR(d, v) (d) = (((d) & ~IPMI_BT_CLR_WR_MASK) | \ -- (((v & 1) << IPMI_BT_CLR_WR_BIT))) - --#define IPMI_BT_CLR_RD_MASK (1 << IPMI_BT_CLR_RD_BIT) - #define IPMI_BT_GET_CLR_RD(d) (((d) >> IPMI_BT_CLR_RD_BIT) & 0x1) --#define IPMI_BT_SET_CLR_RD(d, v) (d) = (((d) & ~IPMI_BT_CLR_RD_MASK) | \ -- (((v & 1) << IPMI_BT_CLR_RD_BIT))) - --#define IPMI_BT_H2B_ATN_MASK (1 << IPMI_BT_H2B_ATN_BIT) - #define IPMI_BT_GET_H2B_ATN(d) (((d) >> IPMI_BT_H2B_ATN_BIT) & 0x1) --#define IPMI_BT_SET_H2B_ATN(d, v) (d) = (((d) & ~IPMI_BT_H2B_ATN_MASK) | \ -- (((v & 1) << IPMI_BT_H2B_ATN_BIT))) - - #define IPMI_BT_B2H_ATN_MASK (1 << IPMI_BT_B2H_ATN_BIT) - #define IPMI_BT_GET_B2H_ATN(d) (((d) >> IPMI_BT_B2H_ATN_BIT) & 0x1) --#define IPMI_BT_SET_B2H_ATN(d, v) (d) = (((d) & ~IPMI_BT_B2H_ATN_MASK) | \ -- (((v & 1) << IPMI_BT_B2H_ATN_BIT))) -+#define IPMI_BT_SET_B2H_ATN(d, v) ((d) = (((d) & ~IPMI_BT_B2H_ATN_MASK) | \ -+ (((v) & 1) << IPMI_BT_B2H_ATN_BIT))) - - #define IPMI_BT_SMS_ATN_MASK (1 << IPMI_BT_SMS_ATN_BIT) - #define IPMI_BT_GET_SMS_ATN(d) (((d) >> IPMI_BT_SMS_ATN_BIT) & 0x1) --#define IPMI_BT_SET_SMS_ATN(d, v) (d) = (((d) & ~IPMI_BT_SMS_ATN_MASK) | \ -- (((v & 1) << IPMI_BT_SMS_ATN_BIT))) -+#define IPMI_BT_SET_SMS_ATN(d, v) ((d) = (((d) & ~IPMI_BT_SMS_ATN_MASK) | \ -+ (((v) & 1) << IPMI_BT_SMS_ATN_BIT))) - - #define IPMI_BT_HBUSY_MASK (1 << IPMI_BT_HBUSY_BIT) - #define IPMI_BT_GET_HBUSY(d) (((d) >> IPMI_BT_HBUSY_BIT) & 0x1) --#define IPMI_BT_SET_HBUSY(d, v) (d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ -- (((v & 1) << IPMI_BT_HBUSY_BIT))) -+#define IPMI_BT_SET_HBUSY(d, v) ((d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ -+ (((v) & 1) << IPMI_BT_HBUSY_BIT))) - - #define IPMI_BT_BBUSY_MASK (1 << IPMI_BT_BBUSY_BIT) --#define IPMI_BT_GET_BBUSY(d) (((d) >> IPMI_BT_BBUSY_BIT) & 0x1) --#define IPMI_BT_SET_BBUSY(d, v) (d) = (((d) & ~IPMI_BT_BBUSY_MASK) | \ -- (((v & 1) << IPMI_BT_BBUSY_BIT))) -+#define IPMI_BT_SET_BBUSY(d, v) ((d) = (((d) & ~IPMI_BT_BBUSY_MASK) | \ -+ (((v) & 1) << IPMI_BT_BBUSY_BIT))) - - - /* Mask register */ -@@ -79,13 +69,13 @@ - - #define IPMI_BT_B2H_IRQ_EN_MASK (1 << IPMI_BT_B2H_IRQ_EN_BIT) - #define IPMI_BT_GET_B2H_IRQ_EN(d) (((d) >> IPMI_BT_B2H_IRQ_EN_BIT) & 0x1) --#define IPMI_BT_SET_B2H_IRQ_EN(d, v) (d) = (((d) & ~IPMI_BT_B2H_IRQ_EN_MASK) | \ -- (((v & 1) << IPMI_BT_B2H_IRQ_EN_BIT))) -+#define IPMI_BT_SET_B2H_IRQ_EN(d, v) ((d) = (((d) & ~IPMI_BT_B2H_IRQ_EN_MASK) |\ -+ (((v) & 1) << IPMI_BT_B2H_IRQ_EN_BIT))) - - #define IPMI_BT_B2H_IRQ_MASK (1 << IPMI_BT_B2H_IRQ_BIT) - #define IPMI_BT_GET_B2H_IRQ(d) (((d) >> IPMI_BT_B2H_IRQ_BIT) & 0x1) --#define IPMI_BT_SET_B2H_IRQ(d, v) (d) = (((d) & ~IPMI_BT_B2H_IRQ_MASK) | \ -- (((v & 1) << IPMI_BT_B2H_IRQ_BIT))) -+#define IPMI_BT_SET_B2H_IRQ(d, v) ((d) = (((d) & ~IPMI_BT_B2H_IRQ_MASK) | \ -+ (((v) & 1) << IPMI_BT_B2H_IRQ_BIT))) - - typedef struct IPMIBT { - IPMIBmc *bmc; --- -2.7.4 - - - -On 12/22/2016 01:18 PM, <email address hidden> wrote: -> From: Corey Minyard <email address hidden> -> -> Macro parameters should almost always have () around them when used. -> llvm reported an error on this. -> -> Remove redundant parenthesis and put parenthesis around the entire -> macros with assignments in case they are used in an expression. -> -> Remove some unused macros. -> -> Reported in https://bugs.launchpad.net/bugs/1651167 -> -> Signed-off-by: Corey Minyard <email address hidden> -> --- -> hw/ipmi/isa_ipmi_bt.c | 34 ++++++++++++---------------------- -> 1 file changed, 12 insertions(+), 22 deletions(-) - -Reviewed-by: Eric Blake <email address hidden> - --- -Eric Blake eblake redhat com +1-919-301-3266 -Libvirt virtualization library http://libvirt.org - - - -From: Corey Minyard <email address hidden> - -Macro parameters should almost always have () around them when used. -llvm reported an error on this. - -Remove redundant parenthesis and put parenthesis around the entire -macros with assignments in case they are used in an expression. - -Remove some unused macros. - -Reported in https://bugs.launchpad.net/bugs/1651167 - -Signed-off-by: Corey Minyard <email address hidden> -Reviewed-by: Eric Blake <email address hidden> ---- - hw/ipmi/isa_ipmi_bt.c | 34 ++++++++++++---------------------- - 1 file changed, 12 insertions(+), 22 deletions(-) - -Changed in v3: - * Add Eric's reviewed-by. Thanks! - -Changes in v2: - * Put parenthesis around macros that had assignment in them. - * Removed some redundant parenthesis. - * Removed some macros that were not used. - -diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c -index f036617..68bf5cd 100644 ---- a/hw/ipmi/isa_ipmi_bt.c -+++ b/hw/ipmi/isa_ipmi_bt.c -@@ -37,40 +37,30 @@ - #define IPMI_BT_HBUSY_BIT 6 - #define IPMI_BT_BBUSY_BIT 7 - --#define IPMI_BT_CLR_WR_MASK (1 << IPMI_BT_CLR_WR_BIT) - #define IPMI_BT_GET_CLR_WR(d) (((d) >> IPMI_BT_CLR_WR_BIT) & 0x1) --#define IPMI_BT_SET_CLR_WR(d, v) (d) = (((d) & ~IPMI_BT_CLR_WR_MASK) | \ -- (((v & 1) << IPMI_BT_CLR_WR_BIT))) - --#define IPMI_BT_CLR_RD_MASK (1 << IPMI_BT_CLR_RD_BIT) - #define IPMI_BT_GET_CLR_RD(d) (((d) >> IPMI_BT_CLR_RD_BIT) & 0x1) --#define IPMI_BT_SET_CLR_RD(d, v) (d) = (((d) & ~IPMI_BT_CLR_RD_MASK) | \ -- (((v & 1) << IPMI_BT_CLR_RD_BIT))) - --#define IPMI_BT_H2B_ATN_MASK (1 << IPMI_BT_H2B_ATN_BIT) - #define IPMI_BT_GET_H2B_ATN(d) (((d) >> IPMI_BT_H2B_ATN_BIT) & 0x1) --#define IPMI_BT_SET_H2B_ATN(d, v) (d) = (((d) & ~IPMI_BT_H2B_ATN_MASK) | \ -- (((v & 1) << IPMI_BT_H2B_ATN_BIT))) - - #define IPMI_BT_B2H_ATN_MASK (1 << IPMI_BT_B2H_ATN_BIT) - #define IPMI_BT_GET_B2H_ATN(d) (((d) >> IPMI_BT_B2H_ATN_BIT) & 0x1) --#define IPMI_BT_SET_B2H_ATN(d, v) (d) = (((d) & ~IPMI_BT_B2H_ATN_MASK) | \ -- (((v & 1) << IPMI_BT_B2H_ATN_BIT))) -+#define IPMI_BT_SET_B2H_ATN(d, v) ((d) = (((d) & ~IPMI_BT_B2H_ATN_MASK) | \ -+ (((v) & 1) << IPMI_BT_B2H_ATN_BIT))) - - #define IPMI_BT_SMS_ATN_MASK (1 << IPMI_BT_SMS_ATN_BIT) - #define IPMI_BT_GET_SMS_ATN(d) (((d) >> IPMI_BT_SMS_ATN_BIT) & 0x1) --#define IPMI_BT_SET_SMS_ATN(d, v) (d) = (((d) & ~IPMI_BT_SMS_ATN_MASK) | \ -- (((v & 1) << IPMI_BT_SMS_ATN_BIT))) -+#define IPMI_BT_SET_SMS_ATN(d, v) ((d) = (((d) & ~IPMI_BT_SMS_ATN_MASK) | \ -+ (((v) & 1) << IPMI_BT_SMS_ATN_BIT))) - - #define IPMI_BT_HBUSY_MASK (1 << IPMI_BT_HBUSY_BIT) - #define IPMI_BT_GET_HBUSY(d) (((d) >> IPMI_BT_HBUSY_BIT) & 0x1) --#define IPMI_BT_SET_HBUSY(d, v) (d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ -- (((v & 1) << IPMI_BT_HBUSY_BIT))) -+#define IPMI_BT_SET_HBUSY(d, v) ((d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ -+ (((v) & 1) << IPMI_BT_HBUSY_BIT))) - - #define IPMI_BT_BBUSY_MASK (1 << IPMI_BT_BBUSY_BIT) --#define IPMI_BT_GET_BBUSY(d) (((d) >> IPMI_BT_BBUSY_BIT) & 0x1) --#define IPMI_BT_SET_BBUSY(d, v) (d) = (((d) & ~IPMI_BT_BBUSY_MASK) | \ -- (((v & 1) << IPMI_BT_BBUSY_BIT))) -+#define IPMI_BT_SET_BBUSY(d, v) ((d) = (((d) & ~IPMI_BT_BBUSY_MASK) | \ -+ (((v) & 1) << IPMI_BT_BBUSY_BIT))) - - - /* Mask register */ -@@ -79,13 +69,13 @@ - - #define IPMI_BT_B2H_IRQ_EN_MASK (1 << IPMI_BT_B2H_IRQ_EN_BIT) - #define IPMI_BT_GET_B2H_IRQ_EN(d) (((d) >> IPMI_BT_B2H_IRQ_EN_BIT) & 0x1) --#define IPMI_BT_SET_B2H_IRQ_EN(d, v) (d) = (((d) & ~IPMI_BT_B2H_IRQ_EN_MASK) | \ -- (((v & 1) << IPMI_BT_B2H_IRQ_EN_BIT))) -+#define IPMI_BT_SET_B2H_IRQ_EN(d, v) ((d) = (((d) & ~IPMI_BT_B2H_IRQ_EN_MASK) |\ -+ (((v) & 1) << IPMI_BT_B2H_IRQ_EN_BIT))) - - #define IPMI_BT_B2H_IRQ_MASK (1 << IPMI_BT_B2H_IRQ_BIT) - #define IPMI_BT_GET_B2H_IRQ(d) (((d) >> IPMI_BT_B2H_IRQ_BIT) & 0x1) --#define IPMI_BT_SET_B2H_IRQ(d, v) (d) = (((d) & ~IPMI_BT_B2H_IRQ_MASK) | \ -- (((v & 1) << IPMI_BT_B2H_IRQ_BIT))) -+#define IPMI_BT_SET_B2H_IRQ(d, v) ((d) = (((d) & ~IPMI_BT_B2H_IRQ_MASK) | \ -+ (((v) & 1) << IPMI_BT_B2H_IRQ_BIT))) - - typedef struct IPMIBT { - IPMIBmc *bmc; --- -2.7.4 - - - -Ping - did this ever get applied? - -On 12/23/2016 08:07 AM, <email address hidden> wrote: -> From: Corey Minyard <email address hidden> -> -> Macro parameters should almost always have () around them when used. -> llvm reported an error on this. -> -> Remove redundant parenthesis and put parenthesis around the entire -> macros with assignments in case they are used in an expression. -> -> Remove some unused macros. -> -> Reported in https://bugs.launchpad.net/bugs/1651167 -> -> Signed-off-by: Corey Minyard <email address hidden> -> Reviewed-by: Eric Blake <email address hidden> -> --- -> hw/ipmi/isa_ipmi_bt.c | 34 ++++++++++++---------------------- -> 1 file changed, 12 insertions(+), 22 deletions(-) -> -> Changed in v3: -> * Add Eric's reviewed-by. Thanks! -> -> Changes in v2: -> * Put parenthesis around macros that had assignment in them. -> * Removed some redundant parenthesis. -> * Removed some macros that were not used. -> -> diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c -> index f036617..68bf5cd 100644 -> --- a/hw/ipmi/isa_ipmi_bt.c -> +++ b/hw/ipmi/isa_ipmi_bt.c -> @@ -37,40 +37,30 @@ -> #define IPMI_BT_HBUSY_BIT 6 -> #define IPMI_BT_BBUSY_BIT 7 -> -> -#define IPMI_BT_CLR_WR_MASK (1 << IPMI_BT_CLR_WR_BIT) -> #define IPMI_BT_GET_CLR_WR(d) (((d) >> IPMI_BT_CLR_WR_BIT) & 0x1) -> -#define IPMI_BT_SET_CLR_WR(d, v) (d) = (((d) & ~IPMI_BT_CLR_WR_MASK) | \ -> - (((v & 1) << IPMI_BT_CLR_WR_BIT))) -> -> -#define IPMI_BT_CLR_RD_MASK (1 << IPMI_BT_CLR_RD_BIT) -> #define IPMI_BT_GET_CLR_RD(d) (((d) >> IPMI_BT_CLR_RD_BIT) & 0x1) -> -#define IPMI_BT_SET_CLR_RD(d, v) (d) = (((d) & ~IPMI_BT_CLR_RD_MASK) | \ -> - (((v & 1) << IPMI_BT_CLR_RD_BIT))) -> -> -#define IPMI_BT_H2B_ATN_MASK (1 << IPMI_BT_H2B_ATN_BIT) -> #define IPMI_BT_GET_H2B_ATN(d) (((d) >> IPMI_BT_H2B_ATN_BIT) & 0x1) -> -#define IPMI_BT_SET_H2B_ATN(d, v) (d) = (((d) & ~IPMI_BT_H2B_ATN_MASK) | \ -> - (((v & 1) << IPMI_BT_H2B_ATN_BIT))) -> -> #define IPMI_BT_B2H_ATN_MASK (1 << IPMI_BT_B2H_ATN_BIT) -> #define IPMI_BT_GET_B2H_ATN(d) (((d) >> IPMI_BT_B2H_ATN_BIT) & 0x1) -> -#define IPMI_BT_SET_B2H_ATN(d, v) (d) = (((d) & ~IPMI_BT_B2H_ATN_MASK) | \ -> - (((v & 1) << IPMI_BT_B2H_ATN_BIT))) -> +#define IPMI_BT_SET_B2H_ATN(d, v) ((d) = (((d) & ~IPMI_BT_B2H_ATN_MASK) | \ -> + (((v) & 1) << IPMI_BT_B2H_ATN_BIT))) -> -> #define IPMI_BT_SMS_ATN_MASK (1 << IPMI_BT_SMS_ATN_BIT) -> #define IPMI_BT_GET_SMS_ATN(d) (((d) >> IPMI_BT_SMS_ATN_BIT) & 0x1) -> -#define IPMI_BT_SET_SMS_ATN(d, v) (d) = (((d) & ~IPMI_BT_SMS_ATN_MASK) | \ -> - (((v & 1) << IPMI_BT_SMS_ATN_BIT))) -> +#define IPMI_BT_SET_SMS_ATN(d, v) ((d) = (((d) & ~IPMI_BT_SMS_ATN_MASK) | \ -> + (((v) & 1) << IPMI_BT_SMS_ATN_BIT))) -> -> #define IPMI_BT_HBUSY_MASK (1 << IPMI_BT_HBUSY_BIT) -> #define IPMI_BT_GET_HBUSY(d) (((d) >> IPMI_BT_HBUSY_BIT) & 0x1) -> -#define IPMI_BT_SET_HBUSY(d, v) (d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ -> - (((v & 1) << IPMI_BT_HBUSY_BIT))) -> +#define IPMI_BT_SET_HBUSY(d, v) ((d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ -> + (((v) & 1) << IPMI_BT_HBUSY_BIT))) -> -> #define IPMI_BT_BBUSY_MASK (1 << IPMI_BT_BBUSY_BIT) -> -#define IPMI_BT_GET_BBUSY(d) (((d) >> IPMI_BT_BBUSY_BIT) & 0x1) -> -#define IPMI_BT_SET_BBUSY(d, v) (d) = (((d) & ~IPMI_BT_BBUSY_MASK) | \ -> - (((v & 1) << IPMI_BT_BBUSY_BIT))) -> +#define IPMI_BT_SET_BBUSY(d, v) ((d) = (((d) & ~IPMI_BT_BBUSY_MASK) | \ -> + (((v) & 1) << IPMI_BT_BBUSY_BIT))) -> -> -> /* Mask register */ -> @@ -79,13 +69,13 @@ -> -> #define IPMI_BT_B2H_IRQ_EN_MASK (1 << IPMI_BT_B2H_IRQ_EN_BIT) -> #define IPMI_BT_GET_B2H_IRQ_EN(d) (((d) >> IPMI_BT_B2H_IRQ_EN_BIT) & 0x1) -> -#define IPMI_BT_SET_B2H_IRQ_EN(d, v) (d) = (((d) & ~IPMI_BT_B2H_IRQ_EN_MASK) | \ -> - (((v & 1) << IPMI_BT_B2H_IRQ_EN_BIT))) -> +#define IPMI_BT_SET_B2H_IRQ_EN(d, v) ((d) = (((d) & ~IPMI_BT_B2H_IRQ_EN_MASK) |\ -> + (((v) & 1) << IPMI_BT_B2H_IRQ_EN_BIT))) -> -> #define IPMI_BT_B2H_IRQ_MASK (1 << IPMI_BT_B2H_IRQ_BIT) -> #define IPMI_BT_GET_B2H_IRQ(d) (((d) >> IPMI_BT_B2H_IRQ_BIT) & 0x1) -> -#define IPMI_BT_SET_B2H_IRQ(d, v) (d) = (((d) & ~IPMI_BT_B2H_IRQ_MASK) | \ -> - (((v & 1) << IPMI_BT_B2H_IRQ_BIT))) -> +#define IPMI_BT_SET_B2H_IRQ(d, v) ((d) = (((d) & ~IPMI_BT_B2H_IRQ_MASK) | \ -> + (((v) & 1) << IPMI_BT_B2H_IRQ_BIT))) -> -> typedef struct IPMIBT { -> IPMIBmc *bmc; -> - --- -Eric Blake eblake redhat com +1-919-301-3266 -Libvirt virtualization library http://libvirt.org - - - -Fix has been included here: -http://git.qemu.org/?p=qemu.git;a=commitdiff;h=cb9a05a4f169347f85 - diff --git a/results/classifier/deepseek-1/output/changes./1882851 b/results/classifier/deepseek-1/output/changes./1882851 deleted file mode 100644 index a1cecc71..00000000 --- a/results/classifier/deepseek-1/output/changes./1882851 +++ /dev/null @@ -1,493 +0,0 @@ - -QEMU video freezes with "Guest disabled display" (virtio driver) - -I am using Arch Linux as my Guest and Host OS, after starting qemu with the following command: - - $ qemu-system-x86_64 -enable-kvm -hda arch-zoom.qcow2 -m 4G -vga virtio - -and waiting for a screen blank, I get this message: - - Guest disabled display - -And nothing happens after that, I can move the mouse or hit any key, and the message is still there. - -I can still reboot the VM but that's not optimal. - -I can reproduce this with the latest QEMU release (5.0.0) or git master, -I also tried this with older releases (4.0.0, 3.0.0) and the issue is still there. - -I can't reproduce this with other video drivers (std, qxl). - -With std/qxl the screen will blank a bit and then continue as normal. - -OK, I found a workaround: sendkey ctrl-alt-f1 from the QEMU console (ctrl alt 2) then I can switch back to X and continue from where I left off. - -Strange, that workaround doesn't work anymore. - -My bad, the workaround works, it's just a bit tricky. - -`xset dpms force off' on the guest is a good way to reproduce it. - -Hmm, happens with xorg only. -Wayland behaves as expected (any mouse/kbd event wakes up the screen). -Which pretty much implies this is a guest bug. - -> Hmm, happens with xorg only. - -Nope, I can reproduce it with sway as well (which is another Wayland compositor). - -To reproduce it with sway, just do: swaymsg "output * dpms off" and then should you see "Guest disabled display", at that point I'm unable to get back image. - -I tried the sendkey ctrl-alt-f2 and then switch back to TTY1 but the "Guest disabled display" remains. - -Can you please tell me which compositor you used? - -Thanks. - -I can't wake up the screen after hitting keys or moving the mouse after turning off the screen with sway. - -Gerd: I tried the LTS kernel on Arch Linux (5.4.46-1-lts) and I can't reproduce the bug with this kernel. - -It works as expected: `xset dpms force off' triggers the "Guest disabled display" and it disappears after moving the mouse. - -Could it be a regression in virtio_gpu? - -I guess I'll try the latest linux git and if it's an issue in master, I'll bisect it. - -I can reproduce it with current linux git master[1]. - -1. git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git - -OK, I was able to bisect, here is the result: - -[diego@arch-zoom linux]$ git bisect bad -3954ff10e06e4fbc548fc02ff1fcaaac3228fed5 is the first bad commit -commit 3954ff10e06e4fbc548fc02ff1fcaaac3228fed5 -Author: Gerd Hoffmann <email address hidden> -Date: Thu Dec 12 13:53:44 2019 +0100 - - drm/virtio: skip set_scanout if framebuffer didn't change - - v2: also check src rect (Chia-I Wu). - - Signed-off-by: Gerd Hoffmann <email address hidden> - Reviewed-by: Chia-I Wu <email address hidden> - Link: http://patchwork<email address hidden> - - drivers/gpu/drm/virtio/virtgpu_plane.c | 35 ++++++++++++++++++++-------------- - 1 file changed, 21 insertions(+), 14 deletions(-) -[diego@arch-zoom linux]$ - - - - -I just sanity checked, and can confirm the bad commit causes it, and going back one commit makes it work. - -When going through a disable/enable cycle without changing the framebuffer -the optimization added by commit 3954ff10e06e causes the screen stay -blank. Add a bool to force an update to fix that. - -Cc: <email address hidden> -Fixes: 3954ff10e06e ("drm/virtio: skip set_scanout if framebuffer didn't change") -Signed-off-by: Gerd Hoffmann <email address hidden> ---- - drivers/gpu/drm/virtio/virtgpu_drv.h | 1 + - drivers/gpu/drm/virtio/virtgpu_display.c | 1 + - drivers/gpu/drm/virtio/virtgpu_plane.c | 4 +++- - 3 files changed, 5 insertions(+), 1 deletion(-) - -diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h -index 7879ff58236f..6d5410d5dd84 100644 ---- a/drivers/gpu/drm/virtio/virtgpu_drv.h -+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h -@@ -138,6 +138,7 @@ struct virtio_gpu_output { - int cur_x; - int cur_y; - bool enabled; -+ bool need_update; - }; - #define drm_crtc_to_virtio_gpu_output(x) \ - container_of(x, struct virtio_gpu_output, crtc) -diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c -index 2b7e6ae65546..44e9c7b874f5 100644 ---- a/drivers/gpu/drm/virtio/virtgpu_display.c -+++ b/drivers/gpu/drm/virtio/virtgpu_display.c -@@ -99,6 +99,7 @@ static void virtio_gpu_crtc_atomic_enable(struct drm_crtc *crtc, - struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc); - - output->enabled = true; -+ output->need_update = true; - } - - static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc, -diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c -index 52d24179bcec..5948031a9ce8 100644 ---- a/drivers/gpu/drm/virtio/virtgpu_plane.c -+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c -@@ -163,7 +163,8 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, - plane->state->src_w != old_state->src_w || - plane->state->src_h != old_state->src_h || - plane->state->src_x != old_state->src_x || -- plane->state->src_y != old_state->src_y) { -+ plane->state->src_y != old_state->src_y || -+ output->need_update) { - DRM_DEBUG("handle 0x%x, crtc %dx%d+%d+%d, src %dx%d+%d+%d\n", - bo->hw_res_handle, - plane->state->crtc_w, plane->state->crtc_h, -@@ -178,6 +179,7 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, - plane->state->src_h >> 16, - plane->state->src_x >> 16, - plane->state->src_y >> 16); -+ output->need_update = false; - } - - virtio_gpu_cmd_resource_flush(vgdev, bo->hw_res_handle, --- -2.18.4 - - - -Gerd, thanks. I can confirm your patch fixes the problem with X, but Wayland (sway) is still affected. - -I tested X and wayland, and while the "Guest disabled display" no longer hangs on X, it still does hangs under wayland. - -Should I bisect again? - -Sway log after the crash. - -It looks like sway requires swayidle to wake up from sleep[1]. - -This works: - -swayidle timeout 2 'swaymsg "output * dpms off"' resume 'swaymsg "output * dpms on"' - -1. https://github.com/swaywm/sway/issues/2914 - -Yeah, I can reproduce the same exact behavior outside of QEMU with sway and it's consistent to what I observed in QEMU. - -> Hmm, happens with xorg only. - -I think you were right all along about this, sorry. - -Thanks for fixing this bug, feel free to close this bug as fixed. - -Will the patch make it for 5.8? - -When going through a disable/enable cycle without changing the -framebuffer the optimization added by commit 3954ff10e06e ("drm/virtio: -skip set_scanout if framebuffer didn't change") causes the screen stay -blank. Add a bool to force an update to fix that. - -Cc: <email address hidden> -Fixes: 3954ff10e06e ("drm/virtio: skip set_scanout if framebuffer didn't change") -Signed-off-by: Gerd Hoffmann <email address hidden> ---- - drivers/gpu/drm/virtio/virtgpu_drv.h | 1 + - drivers/gpu/drm/virtio/virtgpu_display.c | 1 + - drivers/gpu/drm/virtio/virtgpu_plane.c | 4 +++- - 3 files changed, 5 insertions(+), 1 deletion(-) - -diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h -index 9ff9f4ac0522..7b0c319f23c9 100644 ---- a/drivers/gpu/drm/virtio/virtgpu_drv.h -+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h -@@ -138,6 +138,7 @@ struct virtio_gpu_output { - int cur_x; - int cur_y; - bool enabled; -+ bool need_update; - }; - #define drm_crtc_to_virtio_gpu_output(x) \ - container_of(x, struct virtio_gpu_output, crtc) -diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c -index cc7fd957a307..378be5956b30 100644 ---- a/drivers/gpu/drm/virtio/virtgpu_display.c -+++ b/drivers/gpu/drm/virtio/virtgpu_display.c -@@ -100,6 +100,7 @@ static void virtio_gpu_crtc_atomic_enable(struct drm_crtc *crtc, - struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc); - - output->enabled = true; -+ output->need_update = true; - } - - static void virtio_gpu_crtc_atomic_disable(struct drm_crtc *crtc, -diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c -index 52d24179bcec..5948031a9ce8 100644 ---- a/drivers/gpu/drm/virtio/virtgpu_plane.c -+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c -@@ -163,7 +163,8 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, - plane->state->src_w != old_state->src_w || - plane->state->src_h != old_state->src_h || - plane->state->src_x != old_state->src_x || -- plane->state->src_y != old_state->src_y) { -+ plane->state->src_y != old_state->src_y || -+ output->need_update) { - DRM_DEBUG("handle 0x%x, crtc %dx%d+%d+%d, src %dx%d+%d+%d\n", - bo->hw_res_handle, - plane->state->crtc_w, plane->state->crtc_h, -@@ -178,6 +179,7 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, - plane->state->src_h >> 16, - plane->state->src_x >> 16, - plane->state->src_y >> 16); -+ output->need_update = false; - } - - virtio_gpu_cmd_resource_flush(vgdev, bo->hw_res_handle, --- -2.18.4 - - - - Hi, - -> > --- a/drivers/gpu/drm/virtio/virtgpu_display.c -> > +++ b/drivers/gpu/drm/virtio/virtgpu_display.c -> > @@ -100,6 +100,7 @@ static void virtio_gpu_crtc_atomic_enable(struct drm_crtc *crtc, -> > struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc); -> > -> > output->enabled = true; -> > + output->need_update = true; - -> > --- a/drivers/gpu/drm/virtio/virtgpu_plane.c -> > +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c -> > @@ -163,7 +163,8 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, -> > plane->state->src_w != old_state->src_w || -> > plane->state->src_h != old_state->src_h || -> > plane->state->src_x != old_state->src_x || -> > - plane->state->src_y != old_state->src_y) { -> > + plane->state->src_y != old_state->src_y || -> > + output->need_update) { -> -> Uh instead of hand-rolling what's essentially a drm_crtc_needs_modeset -> check, why not use that one? atomic helpers try to keep the usual suspects -> for state transitions already handy, to avoid every driver rolling their -> own. Or do I miss something here? - -Well, the virtio-gpu virtual hardware can't do plane updates and crtc -updates independant from each other. So the crtc callbacks handle -disable only (we don't need a fb for that) and leave the enable to the -plane update. - -I suspect calling drm_atomic_crtc_needs_modeset() in plane update isn't -going to fly ... - -take care, - Gerd - - - -On Mon, Aug 17, 2020 at 11:03:42AM +0200, Gerd Hoffmann wrote: -> Hi, -> -> > > --- a/drivers/gpu/drm/virtio/virtgpu_display.c -> > > +++ b/drivers/gpu/drm/virtio/virtgpu_display.c -> > > @@ -100,6 +100,7 @@ static void virtio_gpu_crtc_atomic_enable(struct drm_crtc *crtc, -> > > struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc); -> > > -> > > output->enabled = true; -> > > + output->need_update = true; -> -> > > --- a/drivers/gpu/drm/virtio/virtgpu_plane.c -> > > +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c -> > > @@ -163,7 +163,8 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, -> > > plane->state->src_w != old_state->src_w || -> > > plane->state->src_h != old_state->src_h || -> > > plane->state->src_x != old_state->src_x || -> > > - plane->state->src_y != old_state->src_y) { -> > > + plane->state->src_y != old_state->src_y || -> > > + output->need_update) { -> > -> > Uh instead of hand-rolling what's essentially a drm_crtc_needs_modeset -> > check, why not use that one? atomic helpers try to keep the usual suspects -> > for state transitions already handy, to avoid every driver rolling their -> > own. Or do I miss something here? -> -> Well, the virtio-gpu virtual hardware can't do plane updates and crtc -> updates independant from each other. So the crtc callbacks handle -> disable only (we don't need a fb for that) and leave the enable to the -> plane update. -> -> I suspect calling drm_atomic_crtc_needs_modeset() in plane update isn't -> going to fly ... - -Digged a bit more, seems crtc_state->*_changed is cleared after modeset -so the following plane update wouldn't see it. Which I think means -there is no way around tracking that in need_update. - -output->enabled is probably not needed though, seems I can replace that -by either output->crtc.state->enable or ->active. Not fully sure which -one, probably ->active. - -take care, - Gerd - - - -When going through a disable/enable cycle without changing the -framebuffer the optimization added by commit 3954ff10e06e ("drm/virtio: -skip set_scanout if framebuffer didn't change") causes the screen stay -blank. Add a bool to force an update to fix that. - -v2: use drm_atomic_crtc_needs_modeset() (Daniel). - -Cc: <email address hidden> -Fixes: 3954ff10e06e ("drm/virtio: skip set_scanout if framebuffer didn't change") -Signed-off-by: Gerd Hoffmann <email address hidden> ---- - drivers/gpu/drm/virtio/virtgpu_drv.h | 1 + - drivers/gpu/drm/virtio/virtgpu_display.c | 11 +++++++++++ - drivers/gpu/drm/virtio/virtgpu_plane.c | 4 +++- - 3 files changed, 15 insertions(+), 1 deletion(-) - -diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h -index 9ff9f4ac0522..4ab1b0ba2925 100644 ---- a/drivers/gpu/drm/virtio/virtgpu_drv.h -+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h -@@ -138,6 +138,7 @@ struct virtio_gpu_output { - int cur_x; - int cur_y; - bool enabled; -+ bool needs_modeset; - }; - #define drm_crtc_to_virtio_gpu_output(x) \ - container_of(x, struct virtio_gpu_output, crtc) -diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c -index 2c2742b8d657..6c26b41f4e0d 100644 ---- a/drivers/gpu/drm/virtio/virtgpu_display.c -+++ b/drivers/gpu/drm/virtio/virtgpu_display.c -@@ -123,6 +123,17 @@ static int virtio_gpu_crtc_atomic_check(struct drm_crtc *crtc, - static void virtio_gpu_crtc_atomic_flush(struct drm_crtc *crtc, - struct drm_crtc_state *old_state) - { -+ struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc); -+ -+ /* -+ * virtio-gpu can't do modeset and plane update operations -+ * independant from each other. So the actual modeset happens -+ * in the plane update callback, and here we just check -+ * whenever we must force the modeset. -+ */ -+ if (drm_atomic_crtc_needs_modeset(crtc->state)) { -+ output->needs_modeset = true; -+ } - } - - static const struct drm_crtc_helper_funcs virtio_gpu_crtc_helper_funcs = { -diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c -index 52d24179bcec..65757409d9ed 100644 ---- a/drivers/gpu/drm/virtio/virtgpu_plane.c -+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c -@@ -163,7 +163,9 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, - plane->state->src_w != old_state->src_w || - plane->state->src_h != old_state->src_h || - plane->state->src_x != old_state->src_x || -- plane->state->src_y != old_state->src_y) { -+ plane->state->src_y != old_state->src_y || -+ output->needs_modeset) { -+ output->needs_modeset = false; - DRM_DEBUG("handle 0x%x, crtc %dx%d+%d+%d, src %dx%d+%d+%d\n", - bo->hw_res_handle, - plane->state->crtc_w, plane->state->crtc_h, --- -2.18.4 - - - -From: Gerd Hoffmann <email address hidden> - -When going through a disable/enable cycle without changing the -framebuffer the optimization added by commit 3954ff10e06e ("drm/virtio: -skip set_scanout if framebuffer didn't change") causes the screen stay -blank. Add a bool to force an update to fix that. - -v2: use drm_atomic_crtc_needs_modeset() (Daniel). - -Cc: <email address hidden> -Fixes: 3954ff10e06e ("drm/virtio: skip set_scanout if framebuffer didn't change") -Signed-off-by: Gerd Hoffmann <email address hidden> -Tested-by: Jiri Slaby <email address hidden> -Tested-by: Diego Viola <email address hidden> ---- - drivers/gpu/drm/virtio/virtgpu_display.c | 11 +++++++++++ - drivers/gpu/drm/virtio/virtgpu_drv.h | 1 + - drivers/gpu/drm/virtio/virtgpu_plane.c | 4 +++- - 3 files changed, 15 insertions(+), 1 deletion(-) - -diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c -index af55b334be2f..35b5c80f5d85 100644 ---- a/drivers/gpu/drm/virtio/virtgpu_display.c -+++ b/drivers/gpu/drm/virtio/virtgpu_display.c -@@ -123,6 +123,17 @@ static int virtio_gpu_crtc_atomic_check(struct drm_crtc *crtc, - static void virtio_gpu_crtc_atomic_flush(struct drm_crtc *crtc, - struct drm_crtc_state *old_state) - { -+ struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc); -+ -+ /* -+ * virtio-gpu can't do modeset and plane update operations -+ * independant from each other. So the actual modeset happens -+ * in the plane update callback, and here we just check -+ * whenever we must force the modeset. -+ */ -+ if (drm_atomic_crtc_needs_modeset(crtc->state)) { -+ output->needs_modeset = true; -+ } - } - - static const struct drm_crtc_helper_funcs virtio_gpu_crtc_helper_funcs = { -diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h -index 9ff9f4ac0522..4ab1b0ba2925 100644 ---- a/drivers/gpu/drm/virtio/virtgpu_drv.h -+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h -@@ -138,6 +138,7 @@ struct virtio_gpu_output { - int cur_x; - int cur_y; - bool enabled; -+ bool needs_modeset; - }; - #define drm_crtc_to_virtio_gpu_output(x) \ - container_of(x, struct virtio_gpu_output, crtc) -diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c -index 52d24179bcec..65757409d9ed 100644 ---- a/drivers/gpu/drm/virtio/virtgpu_plane.c -+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c -@@ -163,7 +163,9 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, - plane->state->src_w != old_state->src_w || - plane->state->src_h != old_state->src_h || - plane->state->src_x != old_state->src_x || -- plane->state->src_y != old_state->src_y) { -+ plane->state->src_y != old_state->src_y || -+ output->needs_modeset) { -+ output->needs_modeset = false; - DRM_DEBUG("handle 0x%x, crtc %dx%d+%d+%d, src %dx%d+%d+%d\n", - bo->hw_res_handle, - plane->state->crtc_w, plane->state->crtc_h, --- -2.28.0 - - - -On Mon, Aug 24, 2020 at 09:24:40AM +0200, Jiri Slaby wrote: -> On 18. 08. 20, 9:25, Gerd Hoffmann wrote: -> > When going through a disable/enable cycle without changing the -> > framebuffer the optimization added by commit 3954ff10e06e ("drm/virtio: -> > skip set_scanout if framebuffer didn't change") causes the screen stay -> > blank. Add a bool to force an update to fix that. -> > -> > v2: use drm_atomic_crtc_needs_modeset() (Daniel). -> > -> > Cc: <email address hidden> -> > Fixes: 3954ff10e06e ("drm/virtio: skip set_scanout if framebuffer didn't change") -> > Signed-off-by: Gerd Hoffmann <email address hidden> -> -> Tested-by: Jiri Slaby <email address hidden> - -Ping. Need an ack or an review to merge this. - -thanks, - Gerd - - - -This bug is now fixed with Linux 5.9-rc5, thank you. - |