| Message ID | 20201027132235.729230758@rtp-net.org |
|---|---|
| State | Superseded |
| Delegated to: | Kever Yang |
| Headers | show |
| Series | rk3399 (Pinebook pro) EDP support | expand |
On 27/10/2020 16:21, Arnaud Patard (Rtp) wrote: > The current code is using an hard coded enum and the of node reg value of > endpoint to find out if the endpoint is mipi/hdmi/lvds/edp/dp. The order > is different between rk3288, rk3399 vop little, rk3399 vop big. > > A possible solution would be to make sure that the rk3288.dtsi and > rk3399.dtsi files have "expected" reg value or an other solution is > to find the kind of endpoint by comparing the endpoint compatible value. > > This patch is implementing the more flexible second solution. > > Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org> > I didn't test individual patches, but non-pinebook-pro parts of this series as a whole (after some more patches of mine) get me a vidconsole on the rk3399-gru-kevin's display, so I'm going to send "Tested-by"s to the patches I had included in my branch: Tested-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Hi Arnaud, Thanks for your patch. Please use module name as subject prefix instead of a file name, eg. "rockchip: video: vop". On 2020/10/27 下午9:21, Arnaud Patard (Rtp) wrote: > The current code is using an hard coded enum and the of node reg value of > endpoint to find out if the endpoint is mipi/hdmi/lvds/edp/dp. The order > is different between rk3288, rk3399 vop little, rk3399 vop big. > > A possible solution would be to make sure that the rk3288.dtsi and > rk3399.dtsi files have "expected" reg value or an other solution is > to find the kind of endpoint by comparing the endpoint compatible value. > > This patch is implementing the more flexible second solution. > > Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org> > > Index: u-boot/arch/arm/include/asm/arch-rockchip/vop_rk3288.h > =================================================================== > --- u-boot.orig/arch/arm/include/asm/arch-rockchip/vop_rk3288.h > +++ u-boot/arch/arm/include/asm/arch-rockchip/vop_rk3288.h For the patch format, please make sure the u-boot is the root directory. Thanks, - Kever > @@ -85,26 +85,13 @@ enum { > LB_RGB_1280X8 = 0x5 > }; > > -#if defined(CONFIG_ROCKCHIP_RK3399) > enum vop_modes { > VOP_MODE_EDP = 0, > VOP_MODE_MIPI, > VOP_MODE_HDMI, > - VOP_MODE_MIPI1, > - VOP_MODE_DP, > - VOP_MODE_NONE, > -}; > -#else > -enum vop_modes { > - VOP_MODE_EDP = 0, > - VOP_MODE_HDMI, > VOP_MODE_LVDS, > - VOP_MODE_MIPI, > - VOP_MODE_NONE, > - VOP_MODE_AUTO_DETECT, > - VOP_MODE_UNKNOWN, > + VOP_MODE_DP, > }; > -#endif > > /* VOP_VERSION_INFO */ > #define M_FPGA_VERSION (0xffff << 16) > Index: u-boot/drivers/video/rockchip/rk_vop.c > =================================================================== > --- u-boot.orig/drivers/video/rockchip/rk_vop.c > +++ u-boot/drivers/video/rockchip/rk_vop.c > @@ -235,12 +235,11 @@ static int rk_display_init(struct udevic > struct clk clk; > enum video_log2_bpp l2bpp; > ofnode remote; > + const char *compat; > > debug("%s(%s, %lu, %s)\n", __func__, > dev_read_name(dev), fbbase, ofnode_get_name(ep_node)); > > - vop_id = ofnode_read_s32_default(ep_node, "reg", -1); > - debug("vop_id=%d\n", vop_id); > ret = ofnode_read_u32(ep_node, "remote-endpoint", &remote_phandle); > if (ret) > return ret; > @@ -282,6 +281,28 @@ static int rk_display_init(struct udevic > if (disp) > break; > }; > + compat = ofnode_get_property(remote, "compatible", NULL); > + if (!compat) { > + debug("%s(%s): Failed to find compatible property\n", > + __func__, dev_read_name(dev)); > + return -EINVAL; > + } > + if (strstr(compat, "edp")) { > + vop_id = VOP_MODE_EDP; > + } else if (strstr(compat, "mipi")) { > + vop_id = VOP_MODE_MIPI; > + } else if (strstr(compat, "hdmi")) { > + vop_id = VOP_MODE_HDMI; > + } else if (strstr(compat, "cdn-dp")) { > + vop_id = VOP_MODE_DP; > + } else if (strstr(compat, "lvds")) { > + vop_id = VOP_MODE_LVDS; > + } else { > + debug("%s(%s): Failed to find vop mode for %s\n", > + __func__, dev_read_name(dev), compat); > + return -EINVAL; > + } > + debug("vop_id=%d\n", vop_id); > > disp_uc_plat = dev_get_uclass_platdata(disp); > debug("Found device '%s', disp_uc_priv=%p\n", disp->name, disp_uc_plat); > > > >
Kever Yang <kever.yang@rock-chips.com> writes: > Hi Arnaud, > > Thanks for your patch. > > Please use module name as subject prefix instead of a file name, > eg. "rockchip: video: vop". > ok. will fix. > On 2020/10/27 下午9:21, Arnaud Patard (Rtp) wrote: >> The current code is using an hard coded enum and the of node reg value of >> endpoint to find out if the endpoint is mipi/hdmi/lvds/edp/dp. The order >> is different between rk3288, rk3399 vop little, rk3399 vop big. >> >> A possible solution would be to make sure that the rk3288.dtsi and >> rk3399.dtsi files have "expected" reg value or an other solution is >> to find the kind of endpoint by comparing the endpoint compatible value. >> >> This patch is implementing the more flexible second solution. >> >> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org> >> >> Index: u-boot/arch/arm/include/asm/arch-rockchip/vop_rk3288.h >> =================================================================== >> --- u-boot.orig/arch/arm/include/asm/arch-rockchip/vop_rk3288.h >> +++ u-boot/arch/arm/include/asm/arch-rockchip/vop_rk3288.h > > For the patch format, please make sure the u-boot is the root directory. > Can you please explain this point ? It's a valid patch which can be applied with patch -p1. I've been generating patches with quilt for ages and patches got merged in various projects including in the kernel even before git, so I fail to get what's wrong with it. Or are you telling me that uboot is accepting only patches generated with git ? Arnaud
On 2020/10/31 上午2:20, Arnaud Patard (Rtp) wrote: > Kever Yang <kever.yang@rock-chips.com> writes: > >> Hi Arnaud, >> >> Thanks for your patch. >> >> Please use module name as subject prefix instead of a file name, >> eg. "rockchip: video: vop". >> > ok. will fix. > >> On 2020/10/27 下午9:21, Arnaud Patard (Rtp) wrote: >>> The current code is using an hard coded enum and the of node reg value of >>> endpoint to find out if the endpoint is mipi/hdmi/lvds/edp/dp. The order >>> is different between rk3288, rk3399 vop little, rk3399 vop big. >>> >>> A possible solution would be to make sure that the rk3288.dtsi and >>> rk3399.dtsi files have "expected" reg value or an other solution is >>> to find the kind of endpoint by comparing the endpoint compatible value. >>> >>> This patch is implementing the more flexible second solution. >>> >>> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org> >>> >>> Index: u-boot/arch/arm/include/asm/arch-rockchip/vop_rk3288.h >>> =================================================================== >>> --- u-boot.orig/arch/arm/include/asm/arch-rockchip/vop_rk3288.h >>> +++ u-boot/arch/arm/include/asm/arch-rockchip/vop_rk3288.h >> For the patch format, please make sure the u-boot is the root directory. >> > Can you please explain this point ? It's a valid patch which can be > applied with patch -p1. I've been generating patches with quilt for ages > and patches got merged in various projects including in the kernel even > before git, so I fail to get what's wrong with it. > Or are you telling me that uboot is accepting only patches generated with git ? Sorry, I though this patch may not able to apply with git am, but after try with it, this patch can be apply directly, them I think you don't need to update this. Thanks, - Kever > > Arnaud > >
Index: u-boot/arch/arm/include/asm/arch-rockchip/vop_rk3288.h =================================================================== --- u-boot.orig/arch/arm/include/asm/arch-rockchip/vop_rk3288.h +++ u-boot/arch/arm/include/asm/arch-rockchip/vop_rk3288.h @@ -85,26 +85,13 @@ enum { LB_RGB_1280X8 = 0x5 }; -#if defined(CONFIG_ROCKCHIP_RK3399) enum vop_modes { VOP_MODE_EDP = 0, VOP_MODE_MIPI, VOP_MODE_HDMI, - VOP_MODE_MIPI1, - VOP_MODE_DP, - VOP_MODE_NONE, -}; -#else -enum vop_modes { - VOP_MODE_EDP = 0, - VOP_MODE_HDMI, VOP_MODE_LVDS, - VOP_MODE_MIPI, - VOP_MODE_NONE, - VOP_MODE_AUTO_DETECT, - VOP_MODE_UNKNOWN, + VOP_MODE_DP, }; -#endif /* VOP_VERSION_INFO */ #define M_FPGA_VERSION (0xffff << 16) Index: u-boot/drivers/video/rockchip/rk_vop.c =================================================================== --- u-boot.orig/drivers/video/rockchip/rk_vop.c +++ u-boot/drivers/video/rockchip/rk_vop.c @@ -235,12 +235,11 @@ static int rk_display_init(struct udevic struct clk clk; enum video_log2_bpp l2bpp; ofnode remote; + const char *compat; debug("%s(%s, %lu, %s)\n", __func__, dev_read_name(dev), fbbase, ofnode_get_name(ep_node)); - vop_id = ofnode_read_s32_default(ep_node, "reg", -1); - debug("vop_id=%d\n", vop_id); ret = ofnode_read_u32(ep_node, "remote-endpoint", &remote_phandle); if (ret) return ret; @@ -282,6 +281,28 @@ static int rk_display_init(struct udevic if (disp) break; }; + compat = ofnode_get_property(remote, "compatible", NULL); + if (!compat) { + debug("%s(%s): Failed to find compatible property\n", + __func__, dev_read_name(dev)); + return -EINVAL; + } + if (strstr(compat, "edp")) { + vop_id = VOP_MODE_EDP; + } else if (strstr(compat, "mipi")) { + vop_id = VOP_MODE_MIPI; + } else if (strstr(compat, "hdmi")) { + vop_id = VOP_MODE_HDMI; + } else if (strstr(compat, "cdn-dp")) { + vop_id = VOP_MODE_DP; + } else if (strstr(compat, "lvds")) { + vop_id = VOP_MODE_LVDS; + } else { + debug("%s(%s): Failed to find vop mode for %s\n", + __func__, dev_read_name(dev), compat); + return -EINVAL; + } + debug("vop_id=%d\n", vop_id); disp_uc_plat = dev_get_uclass_platdata(disp); debug("Found device '%s', disp_uc_priv=%p\n", disp->name, disp_uc_plat);
The current code is using an hard coded enum and the of node reg value of endpoint to find out if the endpoint is mipi/hdmi/lvds/edp/dp. The order is different between rk3288, rk3399 vop little, rk3399 vop big. A possible solution would be to make sure that the rk3288.dtsi and rk3399.dtsi files have "expected" reg value or an other solution is to find the kind of endpoint by comparing the endpoint compatible value. This patch is implementing the more flexible second solution. Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>