Messages in this thread | | | From | CK Hu (胡俊光) <> | Subject | Re: [PATCH v8 08/13] drm/mediatek: gamma: Support multi-bank gamma LUT | Date | Thu, 3 Aug 2023 06:08:31 +0000 |
| |
Hi, Angelo:
On Wed, 2023-08-02 at 10:30 +0200, AngeloGioacchino Del Regno wrote: > Il 02/08/23 06:12, CK Hu (胡俊光) ha scritto: > > Hi, Angelo: > > > > On Tue, 2023-08-01 at 13:58 +0200, AngeloGioacchino Del Regno > > wrote: > > > Newer Gamma IP have got multiple LUT banks: support specifying > > > the > > > size of the LUT banks and handle bank-switching before > > > programming > > > the LUT in mtk_gamma_set_common() in preparation for adding > > > support > > > for MT8195 and newer SoCs. > > > > > > Suggested-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > > > [Angelo: Refactored original commit] > > > Signed-off-by: AngeloGioacchino Del Regno < > > > angelogioacchino.delregno@collabora.com> > > > Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> > > > --- > > > drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 73 +++++++++++++++- > > > --- > > > ---- > > > 1 file changed, 47 insertions(+), 26 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c > > > b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c > > > index a6f7af1a9e8e..fb7c3650a9f7 100644 > > > --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c > > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c > > > @@ -24,6 +24,8 @@ > > > #define DISP_GAMMA_SIZE 0x0030 > > > #define DISP_GAMMA_SIZE_HSIZE GENMASK > > > (28, 16) > > > #define DISP_GAMMA_SIZE_VSIZE GENMASK > > > (12, 0) > > > +#define DISP_GAMMA_BANK 0x0100 > > > +#define DISP_GAMMA_BANK_BANK GENMASK > > > (1, 0) > > > #define DISP_GAMMA_LUT 0x0700 > > > > > > #define DISP_GAMMA_LUT_10BIT_R GENMASK(29, 20) > > > @@ -32,10 +34,12 @@ > > > > > > #define LUT_10BIT_MASK 0x03ff > > > #define LUT_BITS_DEFAULT 10 > > > +#define LUT_BANK_SIZE_DEFAULT 512 > > > > > > struct mtk_disp_gamma_data { > > > bool has_dither; > > > bool lut_diff; > > > + u16 lut_bank_size; > > > u16 lut_size; > > > u8 lut_bits; > > > }; > > > @@ -80,7 +84,9 @@ void mtk_gamma_set_common(struct device *dev, > > > void > > > __iomem *regs, > > > void __iomem *lut_base; > > > bool lut_diff; > > > u8 lut_bits; > > > - u32 cfg_val, word; > > > + u16 lut_bank_size; > > > + u32 cfg_val, lbank_val, word; > > > + int cur_bank, num_lut_banks; > > > > > > /* If there's no gamma lut there's nothing to do here. > > > */ > > > if (!state->gamma_lut) > > > @@ -91,41 +97,54 @@ void mtk_gamma_set_common(struct device *dev, > > > void __iomem *regs, > > > > > > if (gamma && gamma->data) { > > > lut_diff = gamma->data->lut_diff; > > > + lut_bank_size = gamma->data->lut_bank_size; > > > lut_bits = gamma->data->lut_bits; > > > } else { > > > lut_diff = false; > > > + lut_bank_size = LUT_BANK_SIZE_DEFAULT; > > > > LUT_BANK_SIZE_DEFAULT is only for AAL driver, so place it in AAL > > driver > > and pass it into this function. > > > > I completely dislike having a function taking too many parameters. > > Since mtk_gamma_set() in DISP_GAMMA will anyway expand in the future > to > support newer hardware, I'm thinking about just removing AAL support > from > this function and duplicate the few (15 or something) lines of code > in > mtk_disp_aal instead, so that we stop commonizing the mtk_gamma_set > for > both DISP_AAL and DISP_GAMMA. > > Any opinions on that?
OK, let's stop commonizing the mtk_gamma_set ().
Regards, CK
> > Regards, > Angelo > > > Regards, > > CK > > > >
| |