| Message ID | 20171005213842.11423-1-paulo.r.zanoni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 05, 2017 at 09:38:41PM +0000, Paulo Zanoni wrote: > Don't call it when we can do like the other functions and just look at > port->port. Also rename the intel_digital_port variable to make it > look like the other functions. what other functions? Most of our functions use intel_dig_port. Also dport and less used is port. $ grep -r "intel_digital_port \*intel_dig_port" drivers/gpu/drm/i915/ | wc -l 93 $ grep -r "intel_digital_port \*port" drivers/gpu/drm/i915/ | wc -l 11 $ grep -r "intel_digital_port \*dport" drivers/gpu/drm/i915/ | wc -l 18 > > My main goal here is to prevent the copy-pasters from propagating the > call to other parts of the code. > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index ca48bce..6fb90fa 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4614,14 +4614,11 @@ static bool bdw_digital_port_connected(struct drm_i915_private *dev_priv, > } > > static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv, > - struct intel_digital_port *intel_dig_port) > + struct intel_digital_port *port) > { > - struct intel_encoder *intel_encoder = &intel_dig_port->base; > - enum port port; > u32 bit; > > - port = intel_hpd_pin_to_port(intel_encoder->hpd_pin); > - switch (port) { > + switch (port->port) { > case PORT_A: > bit = BXT_DE_PORT_HP_DDIA; > break; > @@ -4632,7 +4629,7 @@ static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv, > bit = BXT_DE_PORT_HP_DDIC; > break; > default: > - MISSING_CASE(port); > + MISSING_CASE(port->port); > return false; > } > > -- > 2.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Em Qui, 2017-10-05 às 14:49 -0700, Rodrigo Vivi escreveu: > On Thu, Oct 05, 2017 at 09:38:41PM +0000, Paulo Zanoni wrote: > > Don't call it when we can do like the other functions and just look > > at > > port->port. Also rename the intel_digital_port variable to make it > > look like the other functions. > > what other functions? All the _digital_port_connected() functions from intel_dp.c. > > Most of our functions use intel_dig_port. Also dport and less used is > port. > > $ grep -r "intel_digital_port \*intel_dig_port" drivers/gpu/drm/i915/ > | wc -l > 93 > $ grep -r "intel_digital_port \*port" drivers/gpu/drm/i915/ | wc -l > 11 > $ grep -r "intel_digital_port \*dport" drivers/gpu/drm/i915/ | wc -l > 18 > > > > > My main goal here is to prevent the copy-pasters from propagating > > the > > call to other parts of the code. > > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > --- > > drivers/gpu/drm/i915/intel_dp.c | 9 +++------ > > 1 file changed, 3 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > b/drivers/gpu/drm/i915/intel_dp.c > > index ca48bce..6fb90fa 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -4614,14 +4614,11 @@ static bool > > bdw_digital_port_connected(struct drm_i915_private *dev_priv, > > } > > > > static bool bxt_digital_port_connected(struct drm_i915_private > > *dev_priv, > > - struct intel_digital_port > > *intel_dig_port) > > + struct intel_digital_port > > *port) > > { > > - struct intel_encoder *intel_encoder = &intel_dig_port- > > >base; > > - enum port port; > > u32 bit; > > > > - port = intel_hpd_pin_to_port(intel_encoder->hpd_pin); > > - switch (port) { > > + switch (port->port) { > > case PORT_A: > > bit = BXT_DE_PORT_HP_DDIA; > > break; > > @@ -4632,7 +4629,7 @@ static bool bxt_digital_port_connected(struct > > drm_i915_private *dev_priv, > > bit = BXT_DE_PORT_HP_DDIC; > > break; > > default: > > - MISSING_CASE(port); > > + MISSING_CASE(port->port); > > return false; > > } > > > > -- > > 2.9.5 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Em Sex, 2017-10-06 às 10:45 +0000, Patchwork escreveu: > == Series Details == > > Series: series starting with [1/2] drm/i915: avoid unnecessary call > to intel_hpd_pin_to_port > URL : https://patchwork.freedesktop.org/series/31459/ > State : warning > > == Summary == > > Series 31459v1 series starting with [1/2] drm/i915: avoid unnecessary > call to intel_hpd_pin_to_port > https://patchwork.freedesktop.org/api/1.0/series/31459/revisions/1/mb > ox/ > > Test chamelium: > Subgroup dp-crc-fast: > pass -> FAIL (fi-kbl-7500u) fdo#102514 > Test gem_ctx_switch: > Subgroup basic-default: > pass -> INCOMPLETE (fi-cnl-y) fdo#103027 > Test gem_exec_suspend: > Subgroup basic-s3: > pass -> DMESG-WARN (fi-cfl-s) fdo#103026 > Subgroup basic-s4-devices: > pass -> DMESG-WARN (fi-kbl-7500u) [ 242.023771] [drm:intel_dp_aux_ch [i915]] *ERROR* dp aux hw did not signal timeout (has irq: 1)! I do not believe this is caused by my patches. This test on this machine is failing in many other recent patch series, but with different error messages. Looks very unstable. > > fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514 > fdo#103027 https://bugs.freedesktop.org/show_bug.cgi?id=103027 > fdo#103026 https://bugs.freedesktop.org/show_bug.cgi?id=103026 > > fi-bdw- > 5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 > time:461s > fi-bdw- > gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 > time:467s > fi-blb- > e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 > time:390s > fi-bsw- > n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 > time:573s > fi-bwr- > 2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 > time:288s > fi-bxt- > dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 > time:526s > fi-bxt- > j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 > time:537s > fi-byt- > j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 > time:538s > fi-byt- > n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 > time:525s > fi-cfl- > s total:289 pass:256 dwarn:1 dfail:0 fail:0 skip:32 > time:553s > fi-cnl- > y total:31 pass:21 dwarn:0 dfail:0 fail:0 skip:9 > fi-elk- > e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 > time:437s > fi-glk- > 1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 > time:599s > fi-hsw- > 4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 > time:439s > fi-hsw- > 4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 > time:416s > fi-ilk- > 650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 > time:468s > fi-ivb- > 3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 > time:508s > fi-ivb- > 3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 > time:479s > fi-kbl- > 7500u total:289 pass:262 dwarn:2 dfail:0 fail:1 skip:24 > time:501s > fi-kbl- > 7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 > time:584s > fi-kbl- > 7567u total:289 pass:265 dwarn:4 dfail:0 fail:0 skip:20 > time:499s > fi-kbl- > r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 > time:598s > fi-pnv- > d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 > time:661s > fi-skl- > 6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 > time:477s > fi-skl- > 6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 > time:663s > fi-skl- > 6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 > time:534s > fi-skl- > 6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 > time:521s > fi-skl- > gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 > time:473s > fi-snb- > 2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 > time:578s > fi-snb- > 2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 > time:437s > > 97c9e99b242fe40bbda48ba2bcaed07c47fba085 drm-tip: 2017y-10m-06d-09h- > 07m-21s UTC integration manifest > d0674fac8e07 drm/i915: avoid division by zero on cnl_calc_wrpll_link > 7d8046f85adf drm/i915: avoid unnecessary call to > intel_hpd_pin_to_port > > == Logs == > > For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchw > ork_5919/ > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Oct 06, 2017 at 06:19:12PM -0300, Paulo Zanoni wrote: > Em Sex, 2017-10-06 às 10:45 +0000, Patchwork escreveu: > > == Series Details == > > > > Series: series starting with [1/2] drm/i915: avoid unnecessary call > > to intel_hpd_pin_to_port > > URL : https://patchwork.freedesktop.org/series/31459/ > > State : warning > > > > == Summary == > > > > Series 31459v1 series starting with [1/2] drm/i915: avoid unnecessary > > call to intel_hpd_pin_to_port > > https://patchwork.freedesktop.org/api/1.0/series/31459/revisions/1/mb > > ox/ > > > > Test chamelium: > > Subgroup dp-crc-fast: > > pass -> FAIL (fi-kbl-7500u) fdo#102514 > > Test gem_ctx_switch: > > Subgroup basic-default: > > pass -> INCOMPLETE (fi-cnl-y) fdo#103027 > > Test gem_exec_suspend: > > Subgroup basic-s3: > > pass -> DMESG-WARN (fi-cfl-s) fdo#103026 > > Subgroup basic-s4-devices: > > pass -> DMESG-WARN (fi-kbl-7500u) > > [ 242.023771] [drm:intel_dp_aux_ch [i915]] *ERROR* dp aux hw did not > signal timeout (has irq: 1)! > > I do not believe this is caused by my patches. This test on this > machine is failing in many other recent patch series, but with > different error messages. Looks very unstable. > > Yes this is the system where we have had these messages due to LSPCON issue recently. Manasi > > > > fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514 > > fdo#103027 https://bugs.freedesktop.org/show_bug.cgi?id=103027 > > fdo#103026 https://bugs.freedesktop.org/show_bug.cgi?id=103026 > > > > fi-bdw- > > 5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 > > time:461s > > fi-bdw- > > gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 > > time:467s > > fi-blb- > > e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 > > time:390s > > fi-bsw- > > n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 > > time:573s > > fi-bwr- > > 2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 > > time:288s > > fi-bxt- > > dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 > > time:526s > > fi-bxt- > > j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 > > time:537s > > fi-byt- > > j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 > > time:538s > > fi-byt- > > n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 > > time:525s > > fi-cfl- > > s total:289 pass:256 dwarn:1 dfail:0 fail:0 skip:32 > > time:553s > > fi-cnl- > > y total:31 pass:21 dwarn:0 dfail:0 fail:0 skip:9 > > fi-elk- > > e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 > > time:437s > > fi-glk- > > 1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 > > time:599s > > fi-hsw- > > 4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 > > time:439s > > fi-hsw- > > 4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 > > time:416s > > fi-ilk- > > 650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 > > time:468s > > fi-ivb- > > 3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 > > time:508s > > fi-ivb- > > 3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 > > time:479s > > fi-kbl- > > 7500u total:289 pass:262 dwarn:2 dfail:0 fail:1 skip:24 > > time:501s > > fi-kbl- > > 7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 > > time:584s > > fi-kbl- > > 7567u total:289 pass:265 dwarn:4 dfail:0 fail:0 skip:20 > > time:499s > > fi-kbl- > > r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 > > time:598s > > fi-pnv- > > d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 > > time:661s > > fi-skl- > > 6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 > > time:477s > > fi-skl- > > 6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 > > time:663s > > fi-skl- > > 6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 > > time:534s > > fi-skl- > > 6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 > > time:521s > > fi-skl- > > gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 > > time:473s > > fi-snb- > > 2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 > > time:578s > > fi-snb- > > 2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 > > time:437s > > > > 97c9e99b242fe40bbda48ba2bcaed07c47fba085 drm-tip: 2017y-10m-06d-09h- > > 07m-21s UTC integration manifest > > d0674fac8e07 drm/i915: avoid division by zero on cnl_calc_wrpll_link > > 7d8046f85adf drm/i915: avoid unnecessary call to > > intel_hpd_pin_to_port > > > > == Logs == > > > > For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchw > > ork_5919/ > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 07/10/17 00:19, Paulo Zanoni wrote: > Em Sex, 2017-10-06 às 10:45 +0000, Patchwork escreveu: >> == Series Details == >> >> Series: series starting with [1/2] drm/i915: avoid unnecessary call >> to intel_hpd_pin_to_port >> URL : https://patchwork.freedesktop.org/series/31459/ >> State : warning >> >> == Summary == >> >> Series 31459v1 series starting with [1/2] drm/i915: avoid unnecessary >> call to intel_hpd_pin_to_port >> https://patchwork.freedesktop.org/api/1.0/series/31459/revisions/1/mb >> ox/ >> >> Test chamelium: >> Subgroup dp-crc-fast: >> pass -> FAIL (fi-kbl-7500u) fdo#102514 >> Test gem_ctx_switch: >> Subgroup basic-default: >> pass -> INCOMPLETE (fi-cnl-y) fdo#103027 >> Test gem_exec_suspend: >> Subgroup basic-s3: >> pass -> DMESG-WARN (fi-cfl-s) fdo#103026 >> Subgroup basic-s4-devices: >> pass -> DMESG-WARN (fi-kbl-7500u) > > [ 242.023771] [drm:intel_dp_aux_ch [i915]] *ERROR* dp aux hw did not > signal timeout (has irq: 1)! > > I do not believe this is caused by my patches. This test on this > machine is failing in many other recent patch series, but with > different error messages. Looks very unstable. Thanks for letting us know! We need more feedback from developers when we get these false positives in pre-merge :) Marta has been taking over the bug filing for a month now. She will know better what to do than me on this. I've Cc:ed her and will let her handle this.
On Thu, 05 Oct 2017, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote: > Don't call it when we can do like the other functions and just look at > port->port. Also rename the intel_digital_port variable to make it > look like the other functions. I guess the question is, *which* other functions. A quick rough grep gives me: $ grep -ho "intel_digital_port[ *]*[a-zA-Z0-9_]*" $(git ls-files -- drivers/gpu/drm/i915) | sort | uniq -c | sort -rn 93 intel_digital_port *intel_dig_port 22 intel_digital_port *dig_port 18 intel_digital_port *dport 11 intel_digital_port *port 5 intel_digital_port_connected 5 intel_digital_port 4 intel_digital_port * 1 intel_digital_port *primary 1 intel_digital_port *irq_port 1 intel_digital_port I think it's more common to use port for enum port. I'm fine with any of the top three for struct intel_dig_port. BR, Jani. > > My main goal here is to prevent the copy-pasters from propagating the > call to other parts of the code. > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index ca48bce..6fb90fa 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4614,14 +4614,11 @@ static bool bdw_digital_port_connected(struct drm_i915_private *dev_priv, > } > > static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv, > - struct intel_digital_port *intel_dig_port) > + struct intel_digital_port *port) > { > - struct intel_encoder *intel_encoder = &intel_dig_port->base; > - enum port port; > u32 bit; > > - port = intel_hpd_pin_to_port(intel_encoder->hpd_pin); > - switch (port) { > + switch (port->port) { > case PORT_A: > bit = BXT_DE_PORT_HP_DDIA; > break; > @@ -4632,7 +4629,7 @@ static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv, > bit = BXT_DE_PORT_HP_DDIC; > break; > default: > - MISSING_CASE(port); > + MISSING_CASE(port->port); > return false; > }
On Fri, 06 Oct 2017, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote: > Em Qui, 2017-10-05 às 14:49 -0700, Rodrigo Vivi escreveu: >> On Thu, Oct 05, 2017 at 09:38:41PM +0000, Paulo Zanoni wrote: >> > Don't call it when we can do like the other functions and just look >> > at >> > port->port. Also rename the intel_digital_port variable to make it >> > look like the other functions. >> >> what other functions? > > All the _digital_port_connected() functions from intel_dp.c. Wah, I should read all of the thread before replying... Anyway, I'd rather move away from port for struct intel_digital_port. BR, Jani. > >> >> Most of our functions use intel_dig_port. Also dport and less used is >> port. >> >> $ grep -r "intel_digital_port \*intel_dig_port" drivers/gpu/drm/i915/ >> | wc -l >> 93 >> $ grep -r "intel_digital_port \*port" drivers/gpu/drm/i915/ | wc -l >> 11 >> $ grep -r "intel_digital_port \*dport" drivers/gpu/drm/i915/ | wc -l >> 18 >> >> > >> > My main goal here is to prevent the copy-pasters from propagating >> > the >> > call to other parts of the code. >> > >> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> >> > --- >> > drivers/gpu/drm/i915/intel_dp.c | 9 +++------ >> > 1 file changed, 3 insertions(+), 6 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/intel_dp.c >> > b/drivers/gpu/drm/i915/intel_dp.c >> > index ca48bce..6fb90fa 100644 >> > --- a/drivers/gpu/drm/i915/intel_dp.c >> > +++ b/drivers/gpu/drm/i915/intel_dp.c >> > @@ -4614,14 +4614,11 @@ static bool >> > bdw_digital_port_connected(struct drm_i915_private *dev_priv, >> > } >> > >> > static bool bxt_digital_port_connected(struct drm_i915_private >> > *dev_priv, >> > - struct intel_digital_port >> > *intel_dig_port) >> > + struct intel_digital_port >> > *port) >> > { >> > - struct intel_encoder *intel_encoder = &intel_dig_port- >> > >base; >> > - enum port port; >> > u32 bit; >> > >> > - port = intel_hpd_pin_to_port(intel_encoder->hpd_pin); >> > - switch (port) { >> > + switch (port->port) { >> > case PORT_A: >> > bit = BXT_DE_PORT_HP_DDIA; >> > break; >> > @@ -4632,7 +4629,7 @@ static bool bxt_digital_port_connected(struct >> > drm_i915_private *dev_priv, >> > bit = BXT_DE_PORT_HP_DDIC; >> > break; >> > default: >> > - MISSING_CASE(port); >> > + MISSING_CASE(port->port); >> > return false; >> > } >> > >> > -- >> > 2.9.5 >> > >> > _______________________________________________ >> > Intel-gfx mailing list >> > Intel-gfx@lists.freedesktop.org >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Oct 05, 2017 at 06:38:41PM -0300, Paulo Zanoni wrote: > Don't call it when we can do like the other functions and just look at > port->port. Also rename the intel_digital_port variable to make it > look like the other functions. > > My main goal here is to prevent the copy-pasters from propagating the > call to other parts of the code. > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index ca48bce..6fb90fa 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4614,14 +4614,11 @@ static bool bdw_digital_port_connected(struct drm_i915_private *dev_priv, > } > > static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv, > - struct intel_digital_port *intel_dig_port) > + struct intel_digital_port *port) > { > - struct intel_encoder *intel_encoder = &intel_dig_port->base; > - enum port port; > u32 bit; > > - port = intel_hpd_pin_to_port(intel_encoder->hpd_pin); > - switch (port) { > + switch (port->port) { Hmm. I'm thinking we might want to go the other way and change everyone else to use hpd_pin instead. IIRC we'll need to start considering mixed hpd_pin<->port mappings with port F. We already had to do this with the BXT A step w/a (though it was just crudely hacked in) which I suppose is the reason the BXT code is still doing this. Though I see no reason why we would have to call intel_hpd_pin_to_port() here. Instead we could just 's/port->port/encoder->hpd_pin/' everywhere. > case PORT_A: > bit = BXT_DE_PORT_HP_DDIA; > break; > @@ -4632,7 +4629,7 @@ static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv, > bit = BXT_DE_PORT_HP_DDIC; > break; > default: > - MISSING_CASE(port); > + MISSING_CASE(port->port); > return false; > } > > -- > 2.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index ca48bce..6fb90fa 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4614,14 +4614,11 @@ static bool bdw_digital_port_connected(struct drm_i915_private *dev_priv, } static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv, - struct intel_digital_port *intel_dig_port) + struct intel_digital_port *port) { - struct intel_encoder *intel_encoder = &intel_dig_port->base; - enum port port; u32 bit; - port = intel_hpd_pin_to_port(intel_encoder->hpd_pin); - switch (port) { + switch (port->port) { case PORT_A: bit = BXT_DE_PORT_HP_DDIA; break; @@ -4632,7 +4629,7 @@ static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv, bit = BXT_DE_PORT_HP_DDIC; break; default: - MISSING_CASE(port); + MISSING_CASE(port->port); return false; }
Don't call it when we can do like the other functions and just look at port->port. Also rename the intel_digital_port variable to make it look like the other functions. My main goal here is to prevent the copy-pasters from propagating the call to other parts of the code. Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)