| Message ID | 20180320062423.9166-2-lucas.demarchi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Mar 19, 2018 at 11:24:17PM -0700, Lucas De Marchi wrote: > This will allow the struct to be embedded in intel_shared_dpll. > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/intel_dpll_mgr.c | 7 ------- > drivers/gpu/drm/i915/intel_dpll_mgr.h | 10 ++++++++++ > 2 files changed, 10 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c > index 51c5ae4e9116..52d6e731c3e9 100644 > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > @@ -1877,13 +1877,6 @@ static void intel_ddi_pll_init(struct drm_device *dev) > } > } > > -struct dpll_info { > - const char *name; > - const int id; > - const struct intel_shared_dpll_funcs *funcs; > - uint32_t flags; > -}; > - > struct intel_dpll_mgr { > const struct dpll_info *dpll_info; > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h > index f24ccf443d25..e99d6385478a 100644 > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h > @@ -205,6 +205,16 @@ struct intel_shared_dpll_funcs { > struct intel_dpll_hw_state *hw_state); > }; > > +/** > + * struct dpll_info - display PLL platform specific info > + */ > +struct dpll_info { > + const char *name; > + const int id; The const here seems pointless. Well, I guess if we go with the copy then it might not be. Although then we get to wonder why 'flags' isn't const. > + const struct intel_shared_dpll_funcs *funcs; > + uint32_t flags; This structure seems to be poorly organized for 64bit machines. > +}; > + > /** > * struct intel_shared_dpll - display PLL with tracked state and users > */ > -- > 2.14.3
On Tue, Mar 20, 2018 at 2:56 AM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Mon, Mar 19, 2018 at 11:24:17PM -0700, Lucas De Marchi wrote: >> This will allow the struct to be embedded in intel_shared_dpll. >> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> >> --- >> drivers/gpu/drm/i915/intel_dpll_mgr.c | 7 ------- >> drivers/gpu/drm/i915/intel_dpll_mgr.h | 10 ++++++++++ >> 2 files changed, 10 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c >> index 51c5ae4e9116..52d6e731c3e9 100644 >> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c >> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c >> @@ -1877,13 +1877,6 @@ static void intel_ddi_pll_init(struct drm_device *dev) >> } >> } >> >> -struct dpll_info { >> - const char *name; >> - const int id; >> - const struct intel_shared_dpll_funcs *funcs; >> - uint32_t flags; >> -}; >> - >> struct intel_dpll_mgr { >> const struct dpll_info *dpll_info; >> >> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h >> index f24ccf443d25..e99d6385478a 100644 >> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h >> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h >> @@ -205,6 +205,16 @@ struct intel_shared_dpll_funcs { >> struct intel_dpll_hw_state *hw_state); >> }; >> >> +/** >> + * struct dpll_info - display PLL platform specific info >> + */ >> +struct dpll_info { >> + const char *name; >> + const int id; > > The const here seems pointless. Well, I guess if we go with the copy > then it might not be. Although then we get to wonder why 'flags' isn't > const. Here I wanted to do just a code move. Any change would be on top. Since it seems the pointer approach is preferred, I'm just going ahead and dropping this. > >> + const struct intel_shared_dpll_funcs *funcs; >> + uint32_t flags; > > This structure seems to be poorly organized for 64bit machines. Yes, there's a 4-bytes right there. Fixing this unfortunately involves changing all tables inside dpll_mgr.c. I'm sending the reordering as the last patch in v2. thanks Lucas De Marchi > >> +}; >> + >> /** >> * struct intel_shared_dpll - display PLL with tracked state and users >> */ >> -- >> 2.14.3 > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Mar 20, 2018 at 02:50:26PM -0700, Lucas De Marchi wrote: > On Tue, Mar 20, 2018 at 2:56 AM, Ville Syrjälä > <ville.syrjala@linux.intel.com> wrote: > > On Mon, Mar 19, 2018 at 11:24:17PM -0700, Lucas De Marchi wrote: > >> This will allow the struct to be embedded in intel_shared_dpll. > >> > >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > >> --- > >> drivers/gpu/drm/i915/intel_dpll_mgr.c | 7 ------- > >> drivers/gpu/drm/i915/intel_dpll_mgr.h | 10 ++++++++++ > >> 2 files changed, 10 insertions(+), 7 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c > >> index 51c5ae4e9116..52d6e731c3e9 100644 > >> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > >> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > >> @@ -1877,13 +1877,6 @@ static void intel_ddi_pll_init(struct drm_device *dev) > >> } > >> } > >> > >> -struct dpll_info { > >> - const char *name; > >> - const int id; > >> - const struct intel_shared_dpll_funcs *funcs; > >> - uint32_t flags; > >> -}; > >> - > >> struct intel_dpll_mgr { > >> const struct dpll_info *dpll_info; > >> > >> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h > >> index f24ccf443d25..e99d6385478a 100644 > >> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h > >> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h > >> @@ -205,6 +205,16 @@ struct intel_shared_dpll_funcs { > >> struct intel_dpll_hw_state *hw_state); > >> }; > >> > >> +/** > >> + * struct dpll_info - display PLL platform specific info > >> + */ > >> +struct dpll_info { > >> + const char *name; > >> + const int id; > > > > The const here seems pointless. Well, I guess if we go with the copy > > then it might not be. Although then we get to wonder why 'flags' isn't > > const. > > Here I wanted to do just a code move. Any change would be on top. Since it seems > the pointer approach is preferred, I'm just going ahead and dropping this. > > > > >> + const struct intel_shared_dpll_funcs *funcs; > >> + uint32_t flags; > > > > This structure seems to be poorly organized for 64bit machines. > > > Yes, there's a 4-bytes right there. Fixing this unfortunately > involves changing > all tables inside dpll_mgr.c. Those aren't using named initializers? > I'm sending the reordering as the last > patch in v2. > > thanks > Lucas De Marchi > > > > >> +}; > >> + > >> /** > >> * struct intel_shared_dpll - display PLL with tracked state and users > >> */ > >> -- > >> 2.14.3 > > > > -- > > Ville Syrjälä > > Intel OTC > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Lucas De Marchi
On Wed, Mar 21, 2018 at 12:25:40PM +0200, Ville Syrjälä wrote: > > > This structure seems to be poorly organized for 64bit machines. > > > > > > Yes, there's a 4-bytes right there. Fixing this unfortunately > > involves changing > > all tables inside dpll_mgr.c. > > Those aren't using named initializers? No. Most of the tables aren't using named initializers. On v2 of the patch set I did the reordering, but without adding named initializers. Lucas De Marchi
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c index 51c5ae4e9116..52d6e731c3e9 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -1877,13 +1877,6 @@ static void intel_ddi_pll_init(struct drm_device *dev) } } -struct dpll_info { - const char *name; - const int id; - const struct intel_shared_dpll_funcs *funcs; - uint32_t flags; -}; - struct intel_dpll_mgr { const struct dpll_info *dpll_info; diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h index f24ccf443d25..e99d6385478a 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h @@ -205,6 +205,16 @@ struct intel_shared_dpll_funcs { struct intel_dpll_hw_state *hw_state); }; +/** + * struct dpll_info - display PLL platform specific info + */ +struct dpll_info { + const char *name; + const int id; + const struct intel_shared_dpll_funcs *funcs; + uint32_t flags; +}; + /** * struct intel_shared_dpll - display PLL with tracked state and users */
This will allow the struct to be embedded in intel_shared_dpll. Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> --- drivers/gpu/drm/i915/intel_dpll_mgr.c | 7 ------- drivers/gpu/drm/i915/intel_dpll_mgr.h | 10 ++++++++++ 2 files changed, 10 insertions(+), 7 deletions(-)