| Message ID | 20220912111814.17466-2-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Some house cleaning | expand |
On Mon, 2022-09-12 at 14:18 +0300, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > No need for the 'procmon' variable here. Just return the correct > thing from the switch statement directly. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- This doesn't make any difference in practice, the compiler will very likely optimize out the procmon variable. In general, I think I think it's preferable to avoid this kind of patches, because they just make git blame a bit harder to interpret. Nevertheless, this is certainly not a reason to nack, so: Reviewed-by: Luca Coelho <luciano.coelho@intel.com> -- Cheers, Luca.
On Mon, 26 Sep 2022, Luca Coelho <luca@coelho.fi> wrote: > On Mon, 2022-09-12 at 14:18 +0300, Ville Syrjala wrote: >> From: Ville Syrjälä <ville.syrjala@linux.intel.com> >> >> No need for the 'procmon' variable here. Just return the correct >> thing from the switch statement directly. >> >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >> --- > > This doesn't make any difference in practice, the compiler will very > likely optimize out the procmon variable. > > In general, I think I think it's preferable to avoid this kind of > patches, because they just make git blame a bit harder to interpret. I think it's nicer to read, ymmv. > > Nevertheless, this is certainly not a reason to nack, so: > > Reviewed-by: Luca Coelho <luciano.coelho@intel.com> N.b. I've already reviewed patches 1-13. [1] BR, Jani. [1] https://lore.kernel.org/r/87fsgw6bs3.fsf@intel.com > > -- > Cheers, > Luca. >
On Mon, 2022-09-26 at 13:43 +0300, Jani Nikula wrote: > On Mon, 26 Sep 2022, Luca Coelho <luca@coelho.fi> wrote: > > On Mon, 2022-09-12 at 14:18 +0300, Ville Syrjala wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > No need for the 'procmon' variable here. Just return the correct > > > thing from the switch statement directly. > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > --- > > > > This doesn't make any difference in practice, the compiler will very > > likely optimize out the procmon variable. > > > > In general, I think I think it's preferable to avoid this kind of > > patches, because they just make git blame a bit harder to interpret. > > I think it's nicer to read, ymmv. Okay, barely. And that's why I commented, I think the churn may not be worth the tiny increase in readability. Anyway, I'm not a maintainer here, so feel free to dismiss my minor comments. > > Nevertheless, this is certainly not a reason to nack, so: > > > > Reviewed-by: Luca Coelho <luciano.coelho@intel.com> > > N.b. I've already reviewed patches 1-13. [1] Oh, I missed that and thought you had only reviewed 6 and 13. Sorry, I'll stop now.
diff --git a/drivers/gpu/drm/i915/display/intel_combo_phy.c b/drivers/gpu/drm/i915/display/intel_combo_phy.c index 64890f39c3cc..71d7aece1dc6 100644 --- a/drivers/gpu/drm/i915/display/intel_combo_phy.c +++ b/drivers/gpu/drm/i915/display/intel_combo_phy.c @@ -53,7 +53,6 @@ static const struct icl_procmon { static const struct icl_procmon * icl_get_procmon_ref_values(struct drm_i915_private *dev_priv, enum phy phy) { - const struct icl_procmon *procmon; u32 val; val = intel_de_read(dev_priv, ICL_PORT_COMP_DW3(phy)); @@ -62,23 +61,16 @@ icl_get_procmon_ref_values(struct drm_i915_private *dev_priv, enum phy phy) MISSING_CASE(val); fallthrough; case VOLTAGE_INFO_0_85V | PROCESS_INFO_DOT_0: - procmon = &icl_procmon_values[PROCMON_0_85V_DOT_0]; - break; + return &icl_procmon_values[PROCMON_0_85V_DOT_0]; case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_0: - procmon = &icl_procmon_values[PROCMON_0_95V_DOT_0]; - break; + return &icl_procmon_values[PROCMON_0_95V_DOT_0]; case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_1: - procmon = &icl_procmon_values[PROCMON_0_95V_DOT_1]; - break; + return &icl_procmon_values[PROCMON_0_95V_DOT_1]; case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_0: - procmon = &icl_procmon_values[PROCMON_1_05V_DOT_0]; - break; + return &icl_procmon_values[PROCMON_1_05V_DOT_0]; case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_1: - procmon = &icl_procmon_values[PROCMON_1_05V_DOT_1]; - break; + return &icl_procmon_values[PROCMON_1_05V_DOT_1]; } - - return procmon; } static void icl_set_procmon_ref_values(struct drm_i915_private *dev_priv,