| Message ID | 1407161709-26369-1-git-send-email-damien.lespiau@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2014-08-04 11:15 GMT-03:00 Damien Lespiau <damien.lespiau@intel.com>: > We don't actually need to write the HDMI entry on DDIs that have no > chance to be used as HDMI ports. > > While this patch shouldn't change the current behaviour, it makes > further enabling work easier as we'll have an eDP table filling the full > 10 entries. > > Suggested-by: Satheeshakrishna M <satheeshakrishna.m@intel.com> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> While your patch looks correct, maybe you could have used dev_priv->vbt.ddi_port_info[port].supports_{dvi,hdmi}, just like we already do in intel_ddi_init. Or you could set another variable at intel_ddi_init and use it, or do some other equivalent check that doesn't require knowledge of what can really go in each port (since we already have it at intel_ddi_init and we probably shouldn't duplicate it to avoid future desync). Anyway, the patch looks correct for now. So if you still think the current approach is the best, you can add Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index ca1f9a8..95933b2 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -163,6 +163,7 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port) > const u32 *ddi_translations_edp; > const u32 *ddi_translations_hdmi; > const u32 *ddi_translations; > + bool need_hdmi = false; > > if (IS_BROADWELL(dev)) { > ddi_translations_fdi = bdw_ddi_translations_fdi; > @@ -195,12 +196,15 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port) > case PORT_B: > case PORT_C: > ddi_translations = ddi_translations_dp; > + need_hdmi = true; > break; > case PORT_D: > if (intel_dp_is_edp(dev, PORT_D)) > ddi_translations = ddi_translations_edp; > - else > + else { > ddi_translations = ddi_translations_dp; > + need_hdmi = true; > + } > break; > case PORT_E: > ddi_translations = ddi_translations_fdi; > @@ -215,6 +219,9 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port) > reg += 4; > } > > + if (!need_hdmi) > + return; > + > /* Choose a good default if VBT is badly populated */ > if (hdmi_level == HDMI_LEVEL_SHIFT_UNKNOWN || > hdmi_level >= n_hdmi_entries) > -- > 1.8.3.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Aug 04, 2014 at 11:28:58AM -0300, Paulo Zanoni wrote: > 2014-08-04 11:15 GMT-03:00 Damien Lespiau <damien.lespiau@intel.com>: > > We don't actually need to write the HDMI entry on DDIs that have no > > chance to be used as HDMI ports. > > > > While this patch shouldn't change the current behaviour, it makes > > further enabling work easier as we'll have an eDP table filling the full > > 10 entries. > > > > Suggested-by: Satheeshakrishna M <satheeshakrishna.m@intel.com> > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > > While your patch looks correct, maybe you could have used > dev_priv->vbt.ddi_port_info[port].supports_{dvi,hdmi}, just like we > already do in intel_ddi_init. Or you could set another variable at > intel_ddi_init and use it, or do some other equivalent check that > doesn't require knowledge of what can really go in each port (since we > already have it at intel_ddi_init and we probably shouldn't duplicate > it to avoid future desync). > > Anyway, the patch looks correct for now. So if you still think the > current approach is the best, you can add Reviewed-by: Paulo Zanoni > <paulo.r.zanoni@intel.com> I actually like that better, Daniel, hold on your horses :)
On Mon, Aug 04, 2014 at 03:44:12PM +0100, Damien Lespiau wrote: > On Mon, Aug 04, 2014 at 11:28:58AM -0300, Paulo Zanoni wrote: > > 2014-08-04 11:15 GMT-03:00 Damien Lespiau <damien.lespiau@intel.com>: > > > We don't actually need to write the HDMI entry on DDIs that have no > > > chance to be used as HDMI ports. > > > > > > While this patch shouldn't change the current behaviour, it makes > > > further enabling work easier as we'll have an eDP table filling the full > > > 10 entries. > > > > > > Suggested-by: Satheeshakrishna M <satheeshakrishna.m@intel.com> > > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > > > > While your patch looks correct, maybe you could have used > > dev_priv->vbt.ddi_port_info[port].supports_{dvi,hdmi}, just like we > > already do in intel_ddi_init. Or you could set another variable at > > intel_ddi_init and use it, or do some other equivalent check that > > doesn't require knowledge of what can really go in each port (since we > > already have it at intel_ddi_init and we probably shouldn't duplicate > > it to avoid future desync). > > > > Anyway, the patch looks correct for now. So if you still think the > > current approach is the best, you can add Reviewed-by: Paulo Zanoni > > <paulo.r.zanoni@intel.com> > > I actually like that better, Daniel, hold on your horses :) So I've spotted some other nice cleanups fly around in the internal m-l, how do those relate to the ones here? Can we please have them for bdw/hsw right away, I liked the added pretty ;-) -Daniel
On Mon, Aug 04, 2014 at 05:04:31PM +0200, Daniel Vetter wrote: > On Mon, Aug 04, 2014 at 03:44:12PM +0100, Damien Lespiau wrote: > > On Mon, Aug 04, 2014 at 11:28:58AM -0300, Paulo Zanoni wrote: > > > 2014-08-04 11:15 GMT-03:00 Damien Lespiau <damien.lespiau@intel.com>: > > > > We don't actually need to write the HDMI entry on DDIs that have no > > > > chance to be used as HDMI ports. > > > > > > > > While this patch shouldn't change the current behaviour, it makes > > > > further enabling work easier as we'll have an eDP table filling the full > > > > 10 entries. > > > > > > > > Suggested-by: Satheeshakrishna M <satheeshakrishna.m@intel.com> > > > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > > > > > > While your patch looks correct, maybe you could have used > > > dev_priv->vbt.ddi_port_info[port].supports_{dvi,hdmi}, just like we > > > already do in intel_ddi_init. Or you could set another variable at > > > intel_ddi_init and use it, or do some other equivalent check that > > > doesn't require knowledge of what can really go in each port (since we > > > already have it at intel_ddi_init and we probably shouldn't duplicate > > > it to avoid future desync). > > > > > > Anyway, the patch looks correct for now. So if you still think the > > > current approach is the best, you can add Reviewed-by: Paulo Zanoni > > > <paulo.r.zanoni@intel.com> > > > > I actually like that better, Daniel, hold on your horses :) > > So I've spotted some other nice cleanups fly around in the internal m-l, > how do those relate to the ones here? Can we please have them for bdw/hsw > right away, I liked the added pretty ;-) We already have all the patches that have been written in tree (excluding this one, which is really anecdotal for current platforms, that will just remove a couple of register writes, a patch is coming this way soon-ish anyway). The remaining clean-ups Sonika and/or I are signed up for are: - rename the DP training vswing/pre-emph defines to not include the nominal values but include the level (as for instance eDP 1.4 introduces low voltage swings and those values makes no sense then) - rename the HSW-specific defines to select the DDI buffer translation slot to just include the slot index as the specifics (vswing/pre-emph) depend on the platform and are starting to be more confusing than helpful.
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index ca1f9a8..95933b2 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -163,6 +163,7 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port) const u32 *ddi_translations_edp; const u32 *ddi_translations_hdmi; const u32 *ddi_translations; + bool need_hdmi = false; if (IS_BROADWELL(dev)) { ddi_translations_fdi = bdw_ddi_translations_fdi; @@ -195,12 +196,15 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port) case PORT_B: case PORT_C: ddi_translations = ddi_translations_dp; + need_hdmi = true; break; case PORT_D: if (intel_dp_is_edp(dev, PORT_D)) ddi_translations = ddi_translations_edp; - else + else { ddi_translations = ddi_translations_dp; + need_hdmi = true; + } break; case PORT_E: ddi_translations = ddi_translations_fdi; @@ -215,6 +219,9 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port) reg += 4; } + if (!need_hdmi) + return; + /* Choose a good default if VBT is badly populated */ if (hdmi_level == HDMI_LEVEL_SHIFT_UNKNOWN || hdmi_level >= n_hdmi_entries)
We don't actually need to write the HDMI entry on DDIs that have no chance to be used as HDMI ports. While this patch shouldn't change the current behaviour, it makes further enabling work easier as we'll have an eDP table filling the full 10 entries. Suggested-by: Satheeshakrishna M <satheeshakrishna.m@intel.com> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> --- drivers/gpu/drm/i915/intel_ddi.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)