[Intel-gfx] [PATCH 3/6] drm/i915/dg1: map/unmap pll clocks

[Lucas De Marchi]

On Thu, Oct 22, 2020 at 04:22:01PM -0700, Matt Roper wrote:
>On Wed, Oct 21, 2020 at 01:20:31AM -0700, Lucas De Marchi wrote:
>> +#define   DG1_DPCLKA_CFGCR0_DDI_CLK_SEL_VAL_TO_ID(val, phy) \
>> +	  ((((val) & DG1_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy)) >> ((phy % 2) * 2)) + (2 * (phy / 2)))
>
>This sure is ugly.  But it looks correct.

Admittedly this deserves a comment on top
/* don't touch, it's correct */

>
>Although the code might wind up being slightly longer, I wonder if it
>would help clarify if we wrote a few at least the last part of this
>expression with ternary operators and symbolic names.  E.g.,
>
>        "... + (phy >= PHY_C ? DPLL_ID_DG1_DPLL2 : DPLL_ID_DG1_DPLL0)"

I will think about

>
>Up to you; the patch looks fine either way.
>
>Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
>

thanks
Lucas De Marchi

>> +
>>  /* CNL PLL */
>>  #define DPLL0_ENABLE		0x46010
>>  #define DPLL1_ENABLE		0x46014
>> --
>> 2.28.0
>>
>
>-- 
>Matt Roper
>Graphics Software Engineer
>VTT-OSGC Platform Enablement
>Intel Corporation
>(916) 356-2795
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx