Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 13 Jan 2021 15:45:03 +0000
From:   Lee Jones <lee.jones@linaro.org>
To:     =?iso-8859-1?Q?Dani=EBl?= Mantione 
        <daniel.mantione@freepascal.org>
Cc:     linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
        linux-fbdev@vger.kernel.org
Subject: Re: [PATCH 14/31] video: fbdev: aty: mach64_ct: Remove some set but
 unused variables
Message-ID: <20210113154503.GD3975472@dell>
References: <20210113145009.1272040-1-lee.jones@linaro.org>
 <20210113145009.1272040-15-lee.jones@linaro.org>
 <alpine.DEB.2.21.2101131618160.8079@idefix.freepascal.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <alpine.DEB.2.21.2101131618160.8079@idefix.freepascal.org>
Precedence: bulk

On Wed, 13 Jan 2021, Daniël Mantione wrote:

> Hi,
> 
> If I remember well, the removed lines have to do with the VGA/accelerator
> mode of the chip. The current driver always runs the chip in accelerator
> mode. Suppose you would want to support high resolution hardware text modes
> with the driver (fbdev bpp=0), then you would need to switch the chip into
> VGA mode mode and then the removed lines become relevant.
> 
> I did some experiments with this when I was working on the driver, but
> because the documentation was silent about the behaviour of extended CRTC
> registers in VGA mode, I failed to make hardware text modes to work
> properly.
> 
> The #if 0 was there so code was already there in case me or someone else
> would pick it up again.

This code has been commented out for *at least* 16 years.

Probably time to let it go. :)

> Best regards,
> 
> Daniël Mantione
> 
> Op Wed, 13 Jan 2021, schreef Lee Jones:
> 
> > Fixes the following W=1 kernel build warning(s):
> > 
> > drivers/video/fbdev/aty/mach64_ct.c: In function ‘aty_init_pll_ct’:
> > drivers/video/fbdev/aty/mach64_ct.c:405:46: warning: variable ‘vga_dsp_on_off’ set but not used [-Wunused-but-set-variable]
> > drivers/video/fbdev/aty/mach64_ct.c:405:30: warning: variable ‘vga_dsp_config’ set but not used [-Wunused-but-set-variable]
> > drivers/video/fbdev/aty/mach64_ct.c:405:18: warning: variable ‘dsp_on_off’ set but not used [-Wunused-but-set-variable]
> > 
> > Cc: daniel.mantione@freepascal.org
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: linux-fbdev@vger.kernel.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> > drivers/video/fbdev/aty/mach64_ct.c | 19 ++-----------------
> > 1 file changed, 2 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/video/fbdev/aty/mach64_ct.c b/drivers/video/fbdev/aty/mach64_ct.c
> > index f87cc81f4fa2b..23eececa1e9d7 100644
> > --- a/drivers/video/fbdev/aty/mach64_ct.c
> > +++ b/drivers/video/fbdev/aty/mach64_ct.c
> > @@ -402,7 +402,7 @@ static int aty_init_pll_ct(const struct fb_info *info, union aty_pll *pll)
> > 	struct atyfb_par *par = (struct atyfb_par *) info->par;
> > 	u8 mpost_div, xpost_div, sclk_post_div_real;
> > 	u32 q, memcntl, trp;
> > -	u32 dsp_config, dsp_on_off, vga_dsp_config, vga_dsp_on_off;
> > +	u32 dsp_config;
> > #ifdef DEBUG
> > 	int pllmclk, pllsclk;
> > #endif
> > @@ -488,25 +488,10 @@ static int aty_init_pll_ct(const struct fb_info *info, union aty_pll *pll)
> > 
> > 	/* Allow BIOS to override */
> > 	dsp_config = aty_ld_le32(DSP_CONFIG, par);
> > -	dsp_on_off = aty_ld_le32(DSP_ON_OFF, par);
> > -	vga_dsp_config = aty_ld_le32(VGA_DSP_CONFIG, par);
> > -	vga_dsp_on_off = aty_ld_le32(VGA_DSP_ON_OFF, par);
> > 
> > 	if (dsp_config)
> > 		pll->ct.dsp_loop_latency = (dsp_config & DSP_LOOP_LATENCY) >> 16;
> > -#if 0
> > -	FIXME: is it relevant for us?
> > -	if ((!dsp_on_off && !M64_HAS(RESET_3D)) ||
> > -		((dsp_on_off == vga_dsp_on_off) &&
> > -		(!dsp_config || !((dsp_config ^ vga_dsp_config) & DSP_XCLKS_PER_QW)))) {
> > -		vga_dsp_on_off &= VGA_DSP_OFF;
> > -		vga_dsp_config &= VGA_DSP_XCLKS_PER_QW;
> > -		if (ATIDivide(vga_dsp_on_off, vga_dsp_config, 5, 1) > 24)
> > -			pll->ct.fifo_size = 32;
> > -		else
> > -			pll->ct.fifo_size = 24;
> > -	}
> > -#endif
> > +
> > 	/* Exit if the user does not want us to tamper with the clock
> > 	rates of her chip. */
> > 	if (par->mclk_per == 0) {


-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog