warning in omap_connector

warning in omap_connector

[Dave Airlie]

Hey Laurent,

I merged drm-misc-next and noticed this, I'm not sure if it's
collateral damage from something else changing or I've just missed it
previously. 32-bit arm build.


/home/airlied/devel/kernel/dim/src/drivers/gpu/drm/omapdrm/omap_connector.c:
In function ‘omap_connector_mode_valid’:
/home/airlied/devel/kernel/dim/src/drivers/gpu/drm/omapdrm/omap_connector.c:92:9:
warning: braces around scalar initializer
  struct drm_display_mode new_mode = { { 0 } };
         ^~~~~~~~~~~~~~~~
/home/airlied/devel/kernel/dim/src/drivers/gpu/drm/omapdrm/omap_connector.c:92:9:
note: (near initialization for ‘new_mode.clock’)

Dave.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: warning in omap_connector

[Ville Syrjälä]

On Tue, Jun 30, 2020 at 04:33:37PM +1000, Dave Airlie wrote:
> Hey Laurent,
> 
> I merged drm-misc-next and noticed this, I'm not sure if it's
> collateral damage from something else changing or I've just missed it
> previously. 32-bit arm build.
> 
> 
> /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/omapdrm/omap_connector.c:
> In function ‘omap_connector_mode_valid’:
> /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/omapdrm/omap_connector.c:92:9:
> warning: braces around scalar initializer
>   struct drm_display_mode new_mode = { { 0 } };

Probably fallout from my drm_display_mode shrinkage.

Going to repeat my usual "just use {} when zero initializing
structs" recommendation. Avoids these stupid warnings, and IMO
also conveys the meaning better since there's no ambiguity
between zero initializing the whole struct vs. zero initializing
just the first member.

>          ^~~~~~~~~~~~~~~~
> /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/omapdrm/omap_connector.c:92:9:
> note: (near initialization for ‘new_mode.clock’)
> 
> Dave.
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: warning in omap_connector

[Alex Deucher]

On Tue, Jun 30, 2020 at 10:15 AM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Tue, Jun 30, 2020 at 04:33:37PM +1000, Dave Airlie wrote:
> > Hey Laurent,
> >
> > I merged drm-misc-next and noticed this, I'm not sure if it's
> > collateral damage from something else changing or I've just missed it
> > previously. 32-bit arm build.
> >
> >
> > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/omapdrm/omap_connector.c:
> > In function ‘omap_connector_mode_valid’:
> > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/omapdrm/omap_connector.c:92:9:
> > warning: braces around scalar initializer
> >   struct drm_display_mode new_mode = { { 0 } };
>
> Probably fallout from my drm_display_mode shrinkage.
>
> Going to repeat my usual "just use {} when zero initializing
> structs" recommendation. Avoids these stupid warnings, and IMO
> also conveys the meaning better since there's no ambiguity
> between zero initializing the whole struct vs. zero initializing
> just the first member.

IIRC, LLVM and GCC treat these slightly differently.  We've generally
just moved to using memset to avoid different compiler complaints when
using these.

Alex

>
> >          ^~~~~~~~~~~~~~~~
> > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/omapdrm/omap_connector.c:92:9:
> > note: (near initialization for ‘new_mode.clock’)
> >
> > Dave.
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: warning in omap_connector

[Ville Syrjälä]

On Tue, Jun 30, 2020 at 10:19:23AM -0400, Alex Deucher wrote:
> On Tue, Jun 30, 2020 at 10:15 AM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >
> > On Tue, Jun 30, 2020 at 04:33:37PM +1000, Dave Airlie wrote:
> > > Hey Laurent,
> > >
> > > I merged drm-misc-next and noticed this, I'm not sure if it's
> > > collateral damage from something else changing or I've just missed it
> > > previously. 32-bit arm build.
> > >
> > >
> > > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/omapdrm/omap_connector.c:
> > > In function ‘omap_connector_mode_valid’:
> > > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/omapdrm/omap_connector.c:92:9:
> > > warning: braces around scalar initializer
> > >   struct drm_display_mode new_mode = { { 0 } };
> >
> > Probably fallout from my drm_display_mode shrinkage.
> >
> > Going to repeat my usual "just use {} when zero initializing
> > structs" recommendation. Avoids these stupid warnings, and IMO
> > also conveys the meaning better since there's no ambiguity
> > between zero initializing the whole struct vs. zero initializing
> > just the first member.
> 
> IIRC, LLVM and GCC treat these slightly differently.  We've generally
> just moved to using memset to avoid different compiler complaints when
> using these.

I don't particularly like memset() since the requirement to
pass the size just adds another way to screw things up. The
usual 'sizeof(*thing)' makes that slightly less of an issue,
but I've noticed that people often don't use that.

Another issue with memset() is that you then can end up with
a block of seemingly random collection of memsets()s between
the variable declarations and the rest of the code. I suppose
if we could declare variables anywhere we could always keep
the two together so it wouldn't look so weird, but can't do
that for the time being. And even with that it would still
lead to less succinct code, which I generally dislike.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: warning in omap_connector

[Laurent Pinchart]

Hi Ville,

On Tue, Jun 30, 2020 at 05:39:02PM +0300, Ville Syrjälä wrote:
> On Tue, Jun 30, 2020 at 10:19:23AM -0400, Alex Deucher wrote:
> > On Tue, Jun 30, 2020 at 10:15 AM Ville Syrjälä wrote:
> > >
> > > On Tue, Jun 30, 2020 at 04:33:37PM +1000, Dave Airlie wrote:
> > > > Hey Laurent,
> > > >
> > > > I merged drm-misc-next and noticed this, I'm not sure if it's
> > > > collateral damage from something else changing or I've just missed it
> > > > previously. 32-bit arm build.
> > > >
> > > >
> > > > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/omapdrm/omap_connector.c:
> > > > In function ‘omap_connector_mode_valid’:
> > > > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/omapdrm/omap_connector.c:92:9:
> > > > warning: braces around scalar initializer
> > > >   struct drm_display_mode new_mode = { { 0 } };
> > >
> > > Probably fallout from my drm_display_mode shrinkage.
> > >
> > > Going to repeat my usual "just use {} when zero initializing
> > > structs" recommendation. Avoids these stupid warnings, and IMO
> > > also conveys the meaning better since there's no ambiguity
> > > between zero initializing the whole struct vs. zero initializing
> > > just the first member.
> > 
> > IIRC, LLVM and GCC treat these slightly differently.  We've generally
> > just moved to using memset to avoid different compiler complaints when
> > using these.
> 
> I don't particularly like memset() since the requirement to
> pass the size just adds another way to screw things up. The
> usual 'sizeof(*thing)' makes that slightly less of an issue,
> but I've noticed that people often don't use that.
> 
> Another issue with memset() is that you then can end up with
> a block of seemingly random collection of memsets()s between
> the variable declarations and the rest of the code. I suppose
> if we could declare variables anywhere we could always keep
> the two together so it wouldn't look so weird, but can't do
> that for the time being. And even with that it would still
> lead to less succinct code, which I generally dislike.

I'd prefer { } over memset, assuming clang and gcc would treat it
correctly. Ville, I can submit a patch, unless you want to do it
yourself as it's a fallout from drm_display_mode shrinkage ;-)
(seriously speaking, not pushing you, I just want to avoid duplicating
work).

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: warning in omap_connector

[Ville Syrjälä]

On Tue, Jun 30, 2020 at 05:41:32PM +0300, Laurent Pinchart wrote:
> Hi Ville,
> 
> On Tue, Jun 30, 2020 at 05:39:02PM +0300, Ville Syrjälä wrote:
> > On Tue, Jun 30, 2020 at 10:19:23AM -0400, Alex Deucher wrote:
> > > On Tue, Jun 30, 2020 at 10:15 AM Ville Syrjälä wrote:
> > > >
> > > > On Tue, Jun 30, 2020 at 04:33:37PM +1000, Dave Airlie wrote:
> > > > > Hey Laurent,
> > > > >
> > > > > I merged drm-misc-next and noticed this, I'm not sure if it's
> > > > > collateral damage from something else changing or I've just missed it
> > > > > previously. 32-bit arm build.
> > > > >
> > > > >
> > > > > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/omapdrm/omap_connector.c:
> > > > > In function ‘omap_connector_mode_valid’:
> > > > > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/omapdrm/omap_connector.c:92:9:
> > > > > warning: braces around scalar initializer
> > > > >   struct drm_display_mode new_mode = { { 0 } };
> > > >
> > > > Probably fallout from my drm_display_mode shrinkage.
> > > >
> > > > Going to repeat my usual "just use {} when zero initializing
> > > > structs" recommendation. Avoids these stupid warnings, and IMO
> > > > also conveys the meaning better since there's no ambiguity
> > > > between zero initializing the whole struct vs. zero initializing
> > > > just the first member.
> > > 
> > > IIRC, LLVM and GCC treat these slightly differently.  We've generally
> > > just moved to using memset to avoid different compiler complaints when
> > > using these.
> > 
> > I don't particularly like memset() since the requirement to
> > pass the size just adds another way to screw things up. The
> > usual 'sizeof(*thing)' makes that slightly less of an issue,
> > but I've noticed that people often don't use that.
> > 
> > Another issue with memset() is that you then can end up with
> > a block of seemingly random collection of memsets()s between
> > the variable declarations and the rest of the code. I suppose
> > if we could declare variables anywhere we could always keep
> > the two together so it wouldn't look so weird, but can't do
> > that for the time being. And even with that it would still
> > lead to less succinct code, which I generally dislike.
> 
> I'd prefer { } over memset, assuming clang and gcc would treat it
> correctly. Ville, I can submit a patch, unless you want to do it
> yourself as it's a fallout from drm_display_mode shrinkage ;-)
> (seriously speaking, not pushing you, I just want to avoid duplicating
> work).

Go ahead if you want to. I'm in middle of a bigger rebase atm
so can't do it right this minute myself.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: warning in omap_connector

[Dave Airlie]

I think I'm still seeing this.

Dave.

On Wed, 1 Jul 2020 at 01:08, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Tue, Jun 30, 2020 at 05:41:32PM +0300, Laurent Pinchart wrote:
> > Hi Ville,
> >
> > On Tue, Jun 30, 2020 at 05:39:02PM +0300, Ville Syrjälä wrote:
> > > On Tue, Jun 30, 2020 at 10:19:23AM -0400, Alex Deucher wrote:
> > > > On Tue, Jun 30, 2020 at 10:15 AM Ville Syrjälä wrote:
> > > > >
> > > > > On Tue, Jun 30, 2020 at 04:33:37PM +1000, Dave Airlie wrote:
> > > > > > Hey Laurent,
> > > > > >
> > > > > > I merged drm-misc-next and noticed this, I'm not sure if it's
> > > > > > collateral damage from something else changing or I've just missed it
> > > > > > previously. 32-bit arm build.
> > > > > >
> > > > > >
> > > > > > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/omapdrm/omap_connector.c:
> > > > > > In function ‘omap_connector_mode_valid’:
> > > > > > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/omapdrm/omap_connector.c:92:9:
> > > > > > warning: braces around scalar initializer
> > > > > >   struct drm_display_mode new_mode = { { 0 } };
> > > > >
> > > > > Probably fallout from my drm_display_mode shrinkage.
> > > > >
> > > > > Going to repeat my usual "just use {} when zero initializing
> > > > > structs" recommendation. Avoids these stupid warnings, and IMO
> > > > > also conveys the meaning better since there's no ambiguity
> > > > > between zero initializing the whole struct vs. zero initializing
> > > > > just the first member.
> > > >
> > > > IIRC, LLVM and GCC treat these slightly differently.  We've generally
> > > > just moved to using memset to avoid different compiler complaints when
> > > > using these.
> > >
> > > I don't particularly like memset() since the requirement to
> > > pass the size just adds another way to screw things up. The
> > > usual 'sizeof(*thing)' makes that slightly less of an issue,
> > > but I've noticed that people often don't use that.
> > >
> > > Another issue with memset() is that you then can end up with
> > > a block of seemingly random collection of memsets()s between
> > > the variable declarations and the rest of the code. I suppose
> > > if we could declare variables anywhere we could always keep
> > > the two together so it wouldn't look so weird, but can't do
> > > that for the time being. And even with that it would still
> > > lead to less succinct code, which I generally dislike.
> >
> > I'd prefer { } over memset, assuming clang and gcc would treat it
> > correctly. Ville, I can submit a patch, unless you want to do it
> > yourself as it's a fallout from drm_display_mode shrinkage ;-)
> > (seriously speaking, not pushing you, I just want to avoid duplicating
> > work).
>
> Go ahead if you want to. I'm in middle of a bigger rebase atm
> so can't do it right this minute myself.
>
> --
> Ville Syrjälä
> Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: warning in omap_connector

[Laurent Pinchart]

Hi Dave,

On Thu, Jul 23, 2020 at 02:28:22PM +1000, Dave Airlie wrote:
> I think I'm still seeing this.

I'm sorry, I've let it slip through the cracks :-S Ville as beaten me at
submitting a patch (kiitos paljon Ville).

> On Wed, 1 Jul 2020 at 01:08, Ville Syrjälä wrote:
> > On Tue, Jun 30, 2020 at 05:41:32PM +0300, Laurent Pinchart wrote:
> > > On Tue, Jun 30, 2020 at 05:39:02PM +0300, Ville Syrjälä wrote:
> > > > On Tue, Jun 30, 2020 at 10:19:23AM -0400, Alex Deucher wrote:
> > > > > On Tue, Jun 30, 2020 at 10:15 AM Ville Syrjälä wrote:
> > > > > >
> > > > > > On Tue, Jun 30, 2020 at 04:33:37PM +1000, Dave Airlie wrote:
> > > > > > > Hey Laurent,
> > > > > > >
> > > > > > > I merged drm-misc-next and noticed this, I'm not sure if it's
> > > > > > > collateral damage from something else changing or I've just missed it
> > > > > > > previously. 32-bit arm build.
> > > > > > >
> > > > > > >
> > > > > > > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/omapdrm/omap_connector.c:
> > > > > > > In function ‘omap_connector_mode_valid’:
> > > > > > > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/omapdrm/omap_connector.c:92:9:
> > > > > > > warning: braces around scalar initializer
> > > > > > >   struct drm_display_mode new_mode = { { 0 } };
> > > > > >
> > > > > > Probably fallout from my drm_display_mode shrinkage.
> > > > > >
> > > > > > Going to repeat my usual "just use {} when zero initializing
> > > > > > structs" recommendation. Avoids these stupid warnings, and IMO
> > > > > > also conveys the meaning better since there's no ambiguity
> > > > > > between zero initializing the whole struct vs. zero initializing
> > > > > > just the first member.
> > > > >
> > > > > IIRC, LLVM and GCC treat these slightly differently.  We've generally
> > > > > just moved to using memset to avoid different compiler complaints when
> > > > > using these.
> > > >
> > > > I don't particularly like memset() since the requirement to
> > > > pass the size just adds another way to screw things up. The
> > > > usual 'sizeof(*thing)' makes that slightly less of an issue,
> > > > but I've noticed that people often don't use that.
> > > >
> > > > Another issue with memset() is that you then can end up with
> > > > a block of seemingly random collection of memsets()s between
> > > > the variable declarations and the rest of the code. I suppose
> > > > if we could declare variables anywhere we could always keep
> > > > the two together so it wouldn't look so weird, but can't do
> > > > that for the time being. And even with that it would still
> > > > lead to less succinct code, which I generally dislike.
> > >
> > > I'd prefer { } over memset, assuming clang and gcc would treat it
> > > correctly. Ville, I can submit a patch, unless you want to do it
> > > yourself as it's a fallout from drm_display_mode shrinkage ;-)
> > > (seriously speaking, not pushing you, I just want to avoid duplicating
> > > work).
> >
> > Go ahead if you want to. I'm in middle of a bigger rebase atm
> > so can't do it right this minute myself.

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel