| Message ID | 1409830075-11139-60-git-send-email-damien.lespiau@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2014-09-04 8:27 GMT-03:00 Damien Lespiau <damien.lespiau@intel.com>: > From: Satheeshakrishna M <satheeshakrishna.m@intel.com> > > Adding structure/enum for SKL clocking implementation. > > v2: Addressed Damien's comment > - Removed internal structure from this header file > > v3: Stove this into the generic intel_dpll_id enum and give them the established > DPLL_ID_ prefixes. (Daniel) > > v4: - We'll only try to share DPLL1/2/3, leaving DPLL0 to eDP > - Use SKL in the skylake shared DPLL names > - Re-add the skl_dpll enum > (Damien) > > v5: Remove SKL_DPLL_NONE (Daniel) > > Signed-off-by: Satheeshakrishna M <satheeshakrishna.m@intel.com> (v2) > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> (v4,v5) > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> (v3) The changes contained on this patch should be part of another patch. I can't really review this if I don't know how those things, especially enum skl_dpll are going to be used. And it also makes the lives of backporters harder. Also, I guess that a patch with 3 signed-off-by stamps shouldn't really need an additional reviewed-by stamp, right? So maybe this should just be merged. > --- > drivers/gpu/drm/i915/i915_drv.h | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 84defa4..65e5ffb 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -206,10 +206,15 @@ enum intel_dpll_id { > /* real shared dpll ids must be >= 0 */ > DPLL_ID_PCH_PLL_A = 0, > DPLL_ID_PCH_PLL_B = 1, > + /* hsw/bdw */ > DPLL_ID_WRPLL1 = 0, > DPLL_ID_WRPLL2 = 1, > + /* skl */ > + DPLL_ID_SKL_DPLL1 = 0, > + DPLL_ID_SKL_DPLL2 = 1, > + DPLL_ID_SKL_DPLL3 = 2, > }; > -#define I915_NUM_PLLS 2 > +#define I915_NUM_PLLS 3 > > struct intel_dpll_hw_state { > /* i9xx, pch plls */ > @@ -243,6 +248,13 @@ struct intel_shared_dpll { > struct intel_dpll_hw_state *hw_state); > }; > > +enum skl_dpll { > + SKL_DPLL0, > + SKL_DPLL1, > + SKL_DPLL2, > + SKL_DPLL3, > +}; > + > /* Used by dp and fdi links */ > struct intel_link_m_n { > uint32_t tu; > -- > 1.8.3.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Sep 22, 2014 at 03:25:30PM -0300, Paulo Zanoni wrote: > 2014-09-04 8:27 GMT-03:00 Damien Lespiau <damien.lespiau@intel.com>: > > From: Satheeshakrishna M <satheeshakrishna.m@intel.com> > > > > Adding structure/enum for SKL clocking implementation. > > > > v2: Addressed Damien's comment > > - Removed internal structure from this header file > > > > v3: Stove this into the generic intel_dpll_id enum and give them the established > > DPLL_ID_ prefixes. (Daniel) > > > > v4: - We'll only try to share DPLL1/2/3, leaving DPLL0 to eDP > > - Use SKL in the skylake shared DPLL names > > - Re-add the skl_dpll enum > > (Damien) > > > > v5: Remove SKL_DPLL_NONE (Daniel) > > > > Signed-off-by: Satheeshakrishna M <satheeshakrishna.m@intel.com> (v2) > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> (v4,v5) > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> (v3) > > The changes contained on this patch should be part of another patch. I > can't really review this if I don't know how those things, especially > enum skl_dpll are going to be used. And it also makes the lives of > backporters harder. > > Also, I guess that a patch with 3 signed-off-by stamps shouldn't > really need an additional reviewed-by stamp, right? So maybe this > should just be merged. Well, I can add my r-b tag here, why not. Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
On Tue, Nov 04, 2014 at 04:12:47PM +0000, Damien Lespiau wrote: > On Mon, Sep 22, 2014 at 03:25:30PM -0300, Paulo Zanoni wrote: > > Also, I guess that a patch with 3 signed-off-by stamps shouldn't > > really need an additional reviewed-by stamp, right? So maybe this > > should just be merged. The sob stamps are because of the rebases in -internal. I've made it practice to not just augment the patch revision log, but also add my name somewhere so that blame can be assigned. But that doesn't mean I've actually reviewed the beast at all complete, just that I've made some adaptions to make it work again on upstream. For this case here where Damien completely rewrote the patch the self-r-b is kinda pointless. So yeah even when the patch has been passed around a lot (and for a long time) and don't think that's a good reason not to throw review at it. Ofc this patch here is really simple ;-) Cheers, Daniel
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 84defa4..65e5ffb 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -206,10 +206,15 @@ enum intel_dpll_id { /* real shared dpll ids must be >= 0 */ DPLL_ID_PCH_PLL_A = 0, DPLL_ID_PCH_PLL_B = 1, + /* hsw/bdw */ DPLL_ID_WRPLL1 = 0, DPLL_ID_WRPLL2 = 1, + /* skl */ + DPLL_ID_SKL_DPLL1 = 0, + DPLL_ID_SKL_DPLL2 = 1, + DPLL_ID_SKL_DPLL3 = 2, }; -#define I915_NUM_PLLS 2 +#define I915_NUM_PLLS 3 struct intel_dpll_hw_state { /* i9xx, pch plls */ @@ -243,6 +248,13 @@ struct intel_shared_dpll { struct intel_dpll_hw_state *hw_state); }; +enum skl_dpll { + SKL_DPLL0, + SKL_DPLL1, + SKL_DPLL2, + SKL_DPLL3, +}; + /* Used by dp and fdi links */ struct intel_link_m_n { uint32_t tu;