[PATCH 0/3] HiBMC driver fixes

[PATCH 0/3] HiBMC driver fixes

[John Garry]

This patchset fixes a couple of issues in probing the HiBMC driver, as
follows:
- fix the probe error path to not carry an error code in the pointer
- don't use invalid legacy fb bpp/depth combination

Another more trivial patch is for using the standard Huawei PCI vendor ID
instead of hard-coding it.

Tested on Huawei D05 board. I can see tux on BMC VGA console.

John Garry (3):
  drm/hisilicon: hibmc: Do not carry error code in HiBMC framebuffer
    pointer
  drm/hisilicon: hibmc: Don't overwrite fb helper surface depth
  drm/hisilicon: hibmc: Use HUAWEI PCI vendor ID macro

 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   | 2 +-
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
1.9.1

[PATCH 1/3] drm/hisilicon: hibmc: Do not carry error code in HiBMC framebuffer pointer

[John Garry]

In hibmc_drm_fb_create(), when the call to hibmc_framebuffer_init() fails
with error, do not store the error code in the HiBMC device frame-buffer
pointer, as this will be later checked for non-zero value in
hibmc_fbdev_destroy() when our intention is to check for a valid function
pointer.

This fixes the following crash:
[    9.699791] Unable to handle kernel NULL pointer dereference at virtual address 000000000000001a
[    9.708672] Mem abort info:
[    9.711489]   ESR = 0x96000004
[    9.714570]   Exception class = DABT (current EL), IL = 32 bits
[    9.720551]   SET = 0, FnV = 0
[    9.723631]   EA = 0, S1PTW = 0
[    9.726799] Data abort info:
[    9.729702]   ISV = 0, ISS = 0x00000004
[    9.733573]   CM = 0, WnR = 0
[    9.736566] [000000000000001a] user address but active_mm is swapper
[    9.742987] Internal error: Oops: 96000004 [#1] PREEMPT SMP
[    9.748614] Modules linked in:
[    9.751694] CPU: 16 PID: 293 Comm: kworker/16:1 Tainted: G        W         4.19.0-rc4-next-20180920-00001-g9b0012c #322
[    9.762681] Hardware name: Huawei Taishan 2280 /D05, BIOS Hisilicon D05 IT21 Nemo 2.0 RC0 04/18/2018
[    9.771915] Workqueue: events work_for_cpu_fn
[    9.776312] pstate: 60000005 (nZCv daif -PAN -UAO)
[    9.781150] pc : drm_mode_object_put+0x0/0x20
[    9.785547] lr : hibmc_fbdev_fini+0x40/0x58
[    9.789767] sp : ffff00000af1bcf0
[    9.793108] x29: ffff00000af1bcf0 x28: 0000000000000000
[    9.798473] x27: 0000000000000000 x26: ffff000008f66630
[    9.803838] x25: 0000000000000000 x24: ffff0000095abb98
[    9.809203] x23: ffff8017db92fe00 x22: ffff8017d2b13000
[    9.814568] x21: ffffffffffffffea x20: ffff8017d2f80018
[    9.819933] x19: ffff8017d28a0018 x18: ffffffffffffffff
[    9.825297] x17: 0000000000000000 x16: 0000000000000000
[    9.830662] x15: ffff0000092296c8 x14: ffff00008939970f
[    9.836026] x13: ffff00000939971d x12: ffff000009229940
[    9.841391] x11: ffff0000085f8fc0 x10: ffff00000af1b9a0
[    9.846756] x9 : 000000000000000d x8 : 6620657a696c6169
[    9.852121] x7 : ffff8017d3340580 x6 : ffff8017d4168000
[    9.857486] x5 : 0000000000000000 x4 : ffff8017db92fb20
[    9.862850] x3 : 0000000000002690 x2 : ffff8017d3340480
[    9.868214] x1 : 0000000000000028 x0 : 0000000000000002
[    9.873580] Process kworker/16:1 (pid: 293, stack limit = 0x(____ptrval____))
[    9.880788] Call trace:
[    9.883252]  drm_mode_object_put+0x0/0x20
[    9.887297]  hibmc_unload+0x1c/0x80
[    9.890815]  hibmc_pci_probe+0x170/0x3c8
[    9.894773]  local_pci_probe+0x3c/0xb0
[    9.898555]  work_for_cpu_fn+0x18/0x28
[    9.902337]  process_one_work+0x1e0/0x318
[    9.906382]  worker_thread+0x228/0x450
[    9.910164]  kthread+0x128/0x130
[    9.913418]  ret_from_fork+0x10/0x18
[    9.917024] Code: a94153f3 a8c27bfd d65f03c0 d503201f (f9400c01)
[    9.923180] ---[ end trace 2695ffa0af5be375 ]---

Fixes: d1667b86795a ("drm/hisilicon/hibmc: Add support for frame buffer")
Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
index b92595c..8bd2907 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
@@ -122,6 +122,7 @@ static int hibmc_drm_fb_create(struct drm_fb_helper *helper,
 	hi_fbdev->fb = hibmc_framebuffer_init(priv->dev, &mode_cmd, gobj);
 	if (IS_ERR(hi_fbdev->fb)) {
 		ret = PTR_ERR(hi_fbdev->fb);
+		hi_fbdev->fb = NULL;
 		DRM_ERROR("failed to initialize framebuffer: %d\n", ret);
 		goto out_release_fbi;
 	}
-- 
1.9.1

[PATCH 2/3] drm/hisilicon: hibmc: Don't overwrite fb helper surface depth

[John Garry]

Currently the driver overwrites the surface depth provided by the fb
helper to give an invalid bpp/surface depth combination.

This has been exposed by commit 70109354fed2 ("drm: Reject unknown legacy
bpp and depth for drm_mode_addfb ioctl"), which now causes the driver to
fail to probe.

Fix by not overwriting the surface depth.

Fixes: d1667b86795a ("drm/hisilicon/hibmc: Add support for frame buffer")
Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
index 8bd2907..edcca17 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
@@ -71,7 +71,6 @@ static int hibmc_drm_fb_create(struct drm_fb_helper *helper,
 	DRM_DEBUG_DRIVER("surface width(%d), height(%d) and bpp(%d)\n",
 			 sizes->surface_width, sizes->surface_height,
 			 sizes->surface_bpp);
-	sizes->surface_depth = 32;
 
 	bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
 
-- 
1.9.1

[PATCH 3/3] drm/hisilicon: hibmc: Use HUAWEI PCI vendor ID macro

[John Garry]

Switch to use Huawei PCI vendor ID macro from pci_ids.h file.

In addition, switch to use PCI_VDEVICE() instead of open coding.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index d4f6f1f..79b6bda 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -402,7 +402,7 @@ static void hibmc_pci_remove(struct pci_dev *pdev)
 }
 
 static struct pci_device_id hibmc_pci_table[] = {
-	{0x19e5, 0x1711, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
+	{ PCI_VDEVICE(HUAWEI, 0x1711) },
 	{0,}
 };
 
-- 
1.9.1

Re: [PATCH 0/3] HiBMC driver fixes

[Xinliang Liu]

Thanks John, good addressing!
The root cause as you said, our hibmc previous frame buffer format
depth setting is wrong which does not pass the new format sanity
checking drm_mode_legacy_fb_format.
For this series,  Reviewed-by: Xinliang Liu <z.liuxinliang@hisilicon.com>
Applied to hisilicon-drm-next.

Thanks,
Xinliang

On Sun, 23 Sep 2018 at 20:32, John Garry <john.garry@huawei.com> wrote:
>
> This patchset fixes a couple of issues in probing the HiBMC driver, as
> follows:
> - fix the probe error path to not carry an error code in the pointer
> - don't use invalid legacy fb bpp/depth combination
>
> Another more trivial patch is for using the standard Huawei PCI vendor ID
> instead of hard-coding it.
>
> Tested on Huawei D05 board. I can see tux on BMC VGA console.
>
> John Garry (3):
>   drm/hisilicon: hibmc: Do not carry error code in HiBMC framebuffer
>     pointer
>   drm/hisilicon: hibmc: Don't overwrite fb helper surface depth
>   drm/hisilicon: hibmc: Use HUAWEI PCI vendor ID macro
>
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   | 2 +-
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] HiBMC driver fixes

[John Garry]

On 26/09/2018 04:00, Xinliang Liu wrote:
> Thanks John, good addressing!
> The root cause as you said, our hibmc previous frame buffer format
> depth setting is wrong which does not pass the new format sanity
> checking drm_mode_legacy_fb_format.
> For this series,  Reviewed-by: Xinliang Liu <z.liuxinliang@hisilicon.com>
> Applied to hisilicon-drm-next.

I can't see this branch in the git associated with this driver from its 
MAINTAINERS entry (git://github.com/xin3liang/linux.git), but please 
ensure these fixes are included in 4.19

Thanks,
John

>
> Thanks,
> Xinliang
>
> On Sun, 23 Sep 2018 at 20:32, John Garry <john.garry@huawei.com> wrote:
>>
>> This patchset fixes a couple of issues in probing the HiBMC driver, as
>> follows:
>> - fix the probe error path to not carry an error code in the pointer
>> - don't use invalid legacy fb bpp/depth combination
>>
>> Another more trivial patch is for using the standard Huawei PCI vendor ID
>> instead of hard-coding it.
>>
>> Tested on Huawei D05 board. I can see tux on BMC VGA console.
>>
>> John Garry (3):
>>   drm/hisilicon: hibmc: Do not carry error code in HiBMC framebuffer
>>     pointer
>>   drm/hisilicon: hibmc: Don't overwrite fb helper surface depth
>>   drm/hisilicon: hibmc: Use HUAWEI PCI vendor ID macro
>>
>>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   | 2 +-
>>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> --
>> 1.9.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> .
>

Re: [PATCH 0/3] HiBMC driver fixes

[Xinliang Liu]

On Wed, 26 Sep 2018 at 16:46, John Garry <john.garry@huawei.com> wrote:
>
> On 26/09/2018 04:00, Xinliang Liu wrote:
> > Thanks John, good addressing!
> > The root cause as you said, our hibmc previous frame buffer format
> > depth setting is wrong which does not pass the new format sanity
> > checking drm_mode_legacy_fb_format.
> > For this series,  Reviewed-by: Xinliang Liu <z.liuxinliang@hisilicon.com>
> > Applied to hisilicon-drm-next.
>
> I can't see this branch in the git associated with this driver from its
> MAINTAINERS entry (git://github.com/xin3liang/linux.git), but please
Not a branch, it is a tag: drm-hisilicon-next-2018-09-26

> ensure these fixes are included in 4.19

As it doesn't affect 4.19-rcx, I send a PULL for 4.20.
See mail "[GIT PULL] drm-hisilicon-next-2018-09-26"

>
> Thanks,
> John
>
> >
> > Thanks,
> > Xinliang
> >
> > On Sun, 23 Sep 2018 at 20:32, John Garry <john.garry@huawei.com> wrote:
> >>
> >> This patchset fixes a couple of issues in probing the HiBMC driver, as
> >> follows:
> >> - fix the probe error path to not carry an error code in the pointer
> >> - don't use invalid legacy fb bpp/depth combination
> >>
> >> Another more trivial patch is for using the standard Huawei PCI vendor ID
> >> instead of hard-coding it.
> >>
> >> Tested on Huawei D05 board. I can see tux on BMC VGA console.
> >>
> >> John Garry (3):
> >>   drm/hisilicon: hibmc: Do not carry error code in HiBMC framebuffer
> >>     pointer
> >>   drm/hisilicon: hibmc: Don't overwrite fb helper surface depth
> >>   drm/hisilicon: hibmc: Use HUAWEI PCI vendor ID macro
> >>
> >>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   | 2 +-
> >>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 2 +-
> >>  2 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >> --
> >> 1.9.1
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > .
> >
>
>

Re: [PATCH 0/3] HiBMC driver fixes

[John Garry]

On 26/09/2018 10:41, Xinliang Liu wrote:
> On Wed, 26 Sep 2018 at 16:46, John Garry <john.garry@huawei.com> wrote:
>>
>> On 26/09/2018 04:00, Xinliang Liu wrote:
>>> Thanks John, good addressing!
>>> The root cause as you said, our hibmc previous frame buffer format
>>> depth setting is wrong which does not pass the new format sanity
>>> checking drm_mode_legacy_fb_format.
>>> For this series,  Reviewed-by: Xinliang Liu <z.liuxinliang@hisilicon.com>
>>> Applied to hisilicon-drm-next.
>>
>> I can't see this branch in the git associated with this driver from its
>> MAINTAINERS entry (git://github.com/xin3liang/linux.git), but please
> Not a branch, it is a tag: drm-hisilicon-next-2018-09-26
>
>> ensure these fixes are included in 4.19
>
> As it doesn't affect 4.19-rcx, I send a PULL for 4.20.
> See mail "[GIT PULL] drm-hisilicon-next-2018-09-26"

When Chris' change goes into 4.20 - which I suspect will be before yours 
- boot-time bisect will be broken.

John

>
>>
>> Thanks,
>> John
>>
>>>
>>> Thanks,
>>> Xinliang
>>>
>>> On Sun, 23 Sep 2018 at 20:32, John Garry <john.garry@huawei.com> wrote:
>>>>
>>>> This patchset fixes a couple of issues in probing the HiBMC driver, as
>>>> follows:
>>>> - fix the probe error path to not carry an error code in the pointer
>>>> - don't use invalid legacy fb bpp/depth combination
>>>>
>>>> Another more trivial patch is for using the standard Huawei PCI vendor ID
>>>> instead of hard-coding it.
>>>>
>>>> Tested on Huawei D05 board. I can see tux on BMC VGA console.
>>>>
>>>> John Garry (3):
>>>>   drm/hisilicon: hibmc: Do not carry error code in HiBMC framebuffer
>>>>     pointer
>>>>   drm/hisilicon: hibmc: Don't overwrite fb helper surface depth
>>>>   drm/hisilicon: hibmc: Use HUAWEI PCI vendor ID macro
>>>>
>>>>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   | 2 +-
>>>>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 2 +-
>>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> --
>>>> 1.9.1
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>> .
>>>
>>
>>
>
> .
>

Re: [PATCH 0/3] HiBMC driver fixes

[Xinliang Liu]

On Wed, 26 Sep 2018 at 17:54, John Garry <john.garry@huawei.com> wrote:
>
> On 26/09/2018 10:41, Xinliang Liu wrote:
> > On Wed, 26 Sep 2018 at 16:46, John Garry <john.garry@huawei.com> wrote:
> >>
> >> On 26/09/2018 04:00, Xinliang Liu wrote:
> >>> Thanks John, good addressing!
> >>> The root cause as you said, our hibmc previous frame buffer format
> >>> depth setting is wrong which does not pass the new format sanity
> >>> checking drm_mode_legacy_fb_format.
> >>> For this series,  Reviewed-by: Xinliang Liu <z.liuxinliang@hisilicon.com>
> >>> Applied to hisilicon-drm-next.
> >>
> >> I can't see this branch in the git associated with this driver from its
> >> MAINTAINERS entry (git://github.com/xin3liang/linux.git), but please
> > Not a branch, it is a tag: drm-hisilicon-next-2018-09-26
> >
> >> ensure these fixes are included in 4.19
> >
> > As it doesn't affect 4.19-rcx, I send a PULL for 4.20.
> > See mail "[GIT PULL] drm-hisilicon-next-2018-09-26"
>
> When Chris' change goes into 4.20 - which I suspect will be before yours
> - boot-time bisect will be broken.

Yeah, that's might be a problem.

>
> John
>
> >
> >>
> >> Thanks,
> >> John
> >>
> >>>
> >>> Thanks,
> >>> Xinliang
> >>>
> >>> On Sun, 23 Sep 2018 at 20:32, John Garry <john.garry@huawei.com> wrote:
> >>>>
> >>>> This patchset fixes a couple of issues in probing the HiBMC driver, as
> >>>> follows:
> >>>> - fix the probe error path to not carry an error code in the pointer
> >>>> - don't use invalid legacy fb bpp/depth combination
> >>>>
> >>>> Another more trivial patch is for using the standard Huawei PCI vendor ID
> >>>> instead of hard-coding it.
> >>>>
> >>>> Tested on Huawei D05 board. I can see tux on BMC VGA console.
> >>>>
> >>>> John Garry (3):
> >>>>   drm/hisilicon: hibmc: Do not carry error code in HiBMC framebuffer
> >>>>     pointer
> >>>>   drm/hisilicon: hibmc: Don't overwrite fb helper surface depth
> >>>>   drm/hisilicon: hibmc: Use HUAWEI PCI vendor ID macro
> >>>>
> >>>>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   | 2 +-
> >>>>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 2 +-
> >>>>  2 files changed, 2 insertions(+), 2 deletions(-)
> >>>>
> >>>> --
> >>>> 1.9.1
> >>>>
> >>>> _______________________________________________
> >>>> dri-devel mailing list
> >>>> dri-devel@lists.freedesktop.org
> >>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>>
> >>> .
> >>>
> >>
> >>
> >
> > .
> >
>
>