Re: [PATCH v2] drm: Remove drm_mode_config::fb_base
On Tue, 2022-10-18 at 23:59 +0300, Andy Shevchenko wrote: > ⚠ External Email > > On Tue, Oct 18, 2022 at 08:26:17PM +, Zack Rusin wrote: > > > On Oct 18, 2022, at 4:20 PM, Andy Shevchenko > > > wrote: > > > On Tue, Oct 18, 2022 at 12:11:51PM -0400, Zack Rusin wrote: > > > > From: Zack Rusin > > > > > > > > v2: Thomas and Laurent noticed that in radeon_fb.c I forgot to set the > > > > info->apertures->ranges[0].base and Laurent noticed a neat little > > > > cleanup > > > > in the hisilicon driver as a result of the drm_mode_config::fb_base > > > > removal. > > > > > > You need to address LKP comment. > > > > That came after v3, and I fixed it in the meantime. I will wait with sending > > v4 until tomorrow to make sure I give everyone time to look at in case > > there’s something else. > > Hmm... Am I missing v3? I answer to v2 and LKP complaint against v1. Possibly, it definitely went to the list, message-id 20221018164929.368012-1-z...@kde.org patchwork sees it at https://patchwork.freedesktop.org/patch/507479/?series=109828&rev=3 lkp email came after I've sent it out. Anyway, it's not a big deal, I've just sent out v4 which should be complete. z
Re: [PATCH v2] drm: Remove drm_mode_config::fb_base
On Tue, Oct 18, 2022 at 08:26:17PM +, Zack Rusin wrote: > > On Oct 18, 2022, at 4:20 PM, Andy Shevchenko > > wrote: > > On Tue, Oct 18, 2022 at 12:11:51PM -0400, Zack Rusin wrote: > >> From: Zack Rusin > >> > >> v2: Thomas and Laurent noticed that in radeon_fb.c I forgot to set the > >> info->apertures->ranges[0].base and Laurent noticed a neat little cleanup > >> in the hisilicon driver as a result of the drm_mode_config::fb_base > >> removal. > > > > You need to address LKP comment. > > That came after v3, and I fixed it in the meantime. I will wait with sending > v4 until tomorrow to make sure I give everyone time to look at in case > there’s something else. Hmm... Am I missing v3? I answer to v2 and LKP complaint against v1. -- With Best Regards, Andy Shevchenko
Re: [PATCH v2] drm: Remove drm_mode_config::fb_base
> On Oct 18, 2022, at 4:20 PM, Andy Shevchenko > wrote: > > ⚠ External Email > > On Tue, Oct 18, 2022 at 12:11:51PM -0400, Zack Rusin wrote: >> From: Zack Rusin >> >> v2: Thomas and Laurent noticed that in radeon_fb.c I forgot to set the >> info->apertures->ranges[0].base and Laurent noticed a neat little cleanup >> in the hisilicon driver as a result of the drm_mode_config::fb_base >> removal. > > You need to address LKP comment. That came after v3, and I fixed it in the meantime. I will wait with sending v4 until tomorrow to make sure I give everyone time to look at in case there’s something else. z
Re: [PATCH v2] drm: Remove drm_mode_config::fb_base
On Tue, Oct 18, 2022 at 12:11:51PM -0400, Zack Rusin wrote: > From: Zack Rusin > > v2: Thomas and Laurent noticed that in radeon_fb.c I forgot to set the > info->apertures->ranges[0].base and Laurent noticed a neat little cleanup > in the hisilicon driver as a result of the drm_mode_config::fb_base > removal. You need to address LKP comment. > The fb_base in struct drm_mode_config has been unused for a long time. > Some drivers set it and some don't leading to a very confusing state > where the variable can't be relied upon, because there's no indication > as to which driver sets it and which doesn't. > > The only usage of fb_base is internal to two drivers so instead of trying > to force it into all the drivers to get it into a coherent state > completely remove it. ... > --- a/drivers/gpu/drm/ast/ast_mode.c > +++ b/drivers/gpu/drm/ast/ast_mode.c > @@ -1778,7 +1778,6 @@ int ast_mode_config_init(struct ast_private *ast) > dev->mode_config.min_width = 0; > dev->mode_config.min_height = 0; > dev->mode_config.preferred_depth = 24; > - dev->mode_config.fb_base = pci_resource_start(pdev, 0); Unused pdev. > if (ast->chip == AST2100 || > ast->chip == AST2200 || I suggest to compile with `make W=1 C=1` on your side before sending v3 and address all compiler complaints. -- With Best Regards, Andy Shevchenko
Re: [PATCH v2] drm: Remove drm_mode_config::fb_base
On Tue, 2022-10-18 at 19:28 +0300, Laurent Pinchart wrote: > > @@ -271,7 +260,8 @@ static int hibmc_load(struct drm_device *dev) > > if (ret) > > goto err; > > > > - ret = drmm_vram_helper_init(dev, pci_resource_start(pdev, 0), priv- > > >fb_size); > > + ret = drmm_vram_helper_init(dev, pci_resource_start(pdev, 0), > > + pci_resource_len(pdev, 1)); > > This should be pci_resource_len(pdev, 0). Apart from that, Eh, yes, of course. Thanks! > Reviewed-by: Laurent Pinchart z
Re: [PATCH v2] drm: Remove drm_mode_config::fb_base
Hi Zack, Thank you for the patch. On Tue, Oct 18, 2022 at 12:11:51PM -0400, Zack Rusin wrote: > From: Zack Rusin > > v2: Thomas and Laurent noticed that in radeon_fb.c I forgot to set the > info->apertures->ranges[0].base and Laurent noticed a neat little cleanup > in the hisilicon driver as a result of the drm_mode_config::fb_base > removal. > > The fb_base in struct drm_mode_config has been unused for a long time. > Some drivers set it and some don't leading to a very confusing state > where the variable can't be relied upon, because there's no indication > as to which driver sets it and which doesn't. > > The only usage of fb_base is internal to two drivers so instead of trying > to force it into all the drivers to get it into a coherent state > completely remove it. > > Signed-off-by: Zack Rusin > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c | 2 -- > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 -- > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 -- > drivers/gpu/drm/amd/amdgpu/dce_v6_0.c| 1 - > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c| 2 -- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c| 2 -- > drivers/gpu/drm/ast/ast_mode.c | 1 - > drivers/gpu/drm/gma500/framebuffer.c | 6 +++--- > drivers/gpu/drm/gma500/psb_drv.h | 1 + > drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 16 +++- > drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 3 --- > drivers/gpu/drm/mgag200/mgag200_mode.c | 1 - > drivers/gpu/drm/msm/msm_fbdev.c | 2 -- > drivers/gpu/drm/nouveau/nouveau_display.c| 1 - > drivers/gpu/drm/nouveau/nv04_fbcon.c | 6 -- > drivers/gpu/drm/omapdrm/omap_fbdev.c | 2 -- > drivers/gpu/drm/qxl/qxl_display.c| 2 -- > drivers/gpu/drm/radeon/radeon_display.c | 2 -- > drivers/gpu/drm/radeon/radeon_fb.c | 2 +- > drivers/gpu/drm/tegra/fb.c | 1 - > drivers/gpu/drm/tiny/bochs.c | 1 - > include/drm/drm_mode_config.h| 2 -- > 22 files changed, 12 insertions(+), 48 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c > index f4b5301ea2a0..09dec2561adf 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c > @@ -498,8 +498,6 @@ static int amdgpu_vkms_sw_init(void *handle) > adev_to_drm(adev)->mode_config.preferred_depth = 24; > adev_to_drm(adev)->mode_config.prefer_shadow = 1; > > - adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base; > - > r = amdgpu_display_modeset_create_props(adev); > if (r) > return r; > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > index 288fce7dc0ed..05051d5d2ec3 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > @@ -2800,8 +2800,6 @@ static int dce_v10_0_sw_init(void *handle) > > adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true; > > - adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base; > - > r = amdgpu_display_modeset_create_props(adev); > if (r) > return r; > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > index cbe5250b31cb..c928bc9eb202 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > @@ -2918,8 +2918,6 @@ static int dce_v11_0_sw_init(void *handle) > > adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true; > > - adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base; > - > r = amdgpu_display_modeset_create_props(adev); > if (r) > return r; > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c > index b1c44fab074f..62315fd5a05f 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c > @@ -2675,7 +2675,6 @@ static int dce_v6_0_sw_init(void *handle) > adev_to_drm(adev)->mode_config.preferred_depth = 24; > adev_to_drm(adev)->mode_config.prefer_shadow = 1; > adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true; > - adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base; > > r = amdgpu_display_modeset_create_props(adev); > if (r) > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > index a22b45c92792..87d5e4c21cb3 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > @@ -2701,8 +2701,6 @@ static int dce_v8_0_sw_init(void *handle) > > adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true; > > - adev_to_drm(adev)->mode_config.fb_base = adev->gmc.ap
