From: Keith Zhao <keith.zhao@starfivetech.com>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-riscv@lists.infradead.org"
<linux-riscv@lists.infradead.org>,
"aou@eecs.berkeley.edu" <aou@eecs.berkeley.edu>,
"suijingfeng@loongson.cn" <suijingfeng@loongson.cn>,
"tzimmermann@suse.de" <tzimmermann@suse.de>,
"paul.walmsley@sifive.com" <paul.walmsley@sifive.com>,
"mripard@kernel.org" <mripard@kernel.org>,
Xingyu Wu <xingyu.wu@starfivetech.com>,
Jack Zhu <jack.zhu@starfivetech.com>,
"palmer@dabbelt.com" <palmer@dabbelt.com>,
"krzysztof.kozlowski+dt@linaro.org"
<krzysztof.kozlowski+dt@linaro.org>,
William Qiu <william.qiu@starfivetech.com>,
Shengyang Chen <shengyang.chen@starfivetech.com>,
Changhuang Liang <changhuang.liang@starfivetech.com>
Subject: RE: [v3 6/6] drm/vs: simple encoder
Date: Thu, 16 May 2024 02:57:44 +0000 [thread overview]
Message-ID: <NTZPR01MB1050BDEAB3E0C1AF28F04FEDEEED2@NTZPR01MB1050.CHNPR01.prod.partner.outlook.cn> (raw)
In-Reply-To: <vimeq7nqco4iu5kbwetkoyiwe4kosb4wf3ch3eqcx4lzexp4hc@cul2sle6glvy>
> -----Original Message-----
> From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Sent: 2024年5月15日 23:17
> To: Keith Zhao <keith.zhao@starfivetech.com>
> Cc: devicetree@vger.kernel.org; dri-devel@lists.freedesktop.org;
> linux-kernel@vger.kernel.org; linux-riscv@lists.infradead.org;
> aou@eecs.berkeley.edu; suijingfeng@loongson.cn; tzimmermann@suse.de;
> paul.walmsley@sifive.com; mripard@kernel.org; Xingyu Wu
> <xingyu.wu@starfivetech.com>; Jack Zhu <jack.zhu@starfivetech.com>;
> palmer@dabbelt.com; krzysztof.kozlowski+dt@linaro.org; William Qiu
> <william.qiu@starfivetech.com>; Shengyang Chen
> <shengyang.chen@starfivetech.com>; Changhuang Liang
> <changhuang.liang@starfivetech.com>
> Subject: Re: [v3 6/6] drm/vs: simple encoder
>
> On Wed, May 15, 2024 at 10:07:27AM +0000, Keith Zhao wrote:
> >
> >
> > > -----Original Message-----
> > > From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > Sent: 2023年12月5日 21:19
> > > To: Keith Zhao <keith.zhao@starfivetech.com>
> > > Cc: devicetree@vger.kernel.org; dri-devel@lists.freedesktop.org;
> > > linux-kernel@vger.kernel.org; linux-riscv@lists.infradead.org;
> > > aou@eecs.berkeley.edu; suijingfeng@loongson.cn; tzimmermann@suse.de;
> > > paul.walmsley@sifive.com; mripard@kernel.org; Xingyu Wu
> > > <xingyu.wu@starfivetech.com>; Jack Zhu <jack.zhu@starfivetech.com>;
> > > palmer@dabbelt.com; krzysztof.kozlowski+dt@linaro.org; William Qiu
> > > <william.qiu@starfivetech.com>; Shengyang Chen
> > > <shengyang.chen@starfivetech.com>; Changhuang Liang
> > > <changhuang.liang@starfivetech.com>
> > > Subject: Re: [v3 6/6] drm/vs: simple encoder
> > >
> > > On Tue, 5 Dec 2023 at 15:14, Dmitry Baryshkov
> > > <dmitry.baryshkov@linaro.org>
> > > wrote:
> > > >
> > > > On Mon, 4 Dec 2023 at 14:33, Keith Zhao
> > > > <keith.zhao@starfivetech.com>
> > > wrote:
> > > > >
> > > > > add simple encoder for dsi bridge
> > > >
> > > > This doesn't look like a proper commit message.
> > > >
> > > > >
> > > > > Signed-off-by: Keith Zhao <keith.zhao@starfivetech.com>
> > > > > ---
> > > > > drivers/gpu/drm/verisilicon/Makefile | 4 +-
> > > > > drivers/gpu/drm/verisilicon/vs_drv.c | 2 +
> > > > > drivers/gpu/drm/verisilicon/vs_simple_enc.c | 195
> > > > > ++++++++++++++++++++
> > > > > ++++++++++++++++++++ drivers/gpu/drm/verisilicon/vs_simple_enc.h
> > > > > ++++++++++++++++++++ |
> > > > > 23 +++
> > > > > 4 files changed, 223 insertions(+), 1 deletion(-) create mode
> > > > > 100644 drivers/gpu/drm/verisilicon/vs_simple_enc.c
> > > > > create mode 100644 drivers/gpu/drm/verisilicon/vs_simple_enc.h
> > > > >
> > > > > diff --git a/drivers/gpu/drm/verisilicon/Makefile
> > > > > b/drivers/gpu/drm/verisilicon/Makefile
> > > > > index 71fadafcee13..cd5d0a90bcfe 100644
> > > > > --- a/drivers/gpu/drm/verisilicon/Makefile
> > > > > +++ b/drivers/gpu/drm/verisilicon/Makefile
> > > > > @@ -5,6 +5,8 @@ vs_drm-objs := vs_dc_hw.o \
> > > > > vs_crtc.o \
> > > > > vs_drv.o \
> > > > > vs_modeset.o \
> > > > > - vs_plane.o
> > > > > + vs_plane.o \
> > > > > + vs_simple_enc.o
> > > > > +
> > > > > vs_drm-$(CONFIG_DRM_VERISILICON_STARFIVE_HDMI) +=
> > > starfive_hdmi.o
> > > > > obj-$(CONFIG_DRM_VERISILICON) += vs_drm.o diff --git
> > > > > a/drivers/gpu/drm/verisilicon/vs_drv.c
> > > > > b/drivers/gpu/drm/verisilicon/vs_drv.c
> > > > > index d7e5199fe293..946f137ab124 100644
> > > > > --- a/drivers/gpu/drm/verisilicon/vs_drv.c
> > > > > +++ b/drivers/gpu/drm/verisilicon/vs_drv.c
> > > > > @@ -23,6 +23,7 @@
> > > > > #include "vs_drv.h"
> > > > > #include "vs_modeset.h"
> > > > > #include "vs_dc.h"
> > > > > +#include "vs_simple_enc.h"
> > > > >
> > > > > #define DRV_NAME "verisilicon"
> > > > > #define DRV_DESC "Verisilicon DRM driver"
> > > > > @@ -217,6 +218,7 @@ static struct platform_driver
> > > > > *drm_sub_drivers[] = { #ifdef
> CONFIG_DRM_VERISILICON_STARFIVE_HDMI
> > > > > &starfive_hdmi_driver,
> > > > > #endif
> > > > > + &simple_encoder_driver,
> > > > > };
> > > > >
> > > > > static struct component_match *vs_drm_match_add(struct device
> > > > > *dev) diff --git a/drivers/gpu/drm/verisilicon/vs_simple_enc.c
> > > > > b/drivers/gpu/drm/verisilicon/vs_simple_enc.c
> > > > > new file mode 100644
> > > > > index 000000000000..c5a8d82bc469
> > > > > --- /dev/null
> > > > > +++ b/drivers/gpu/drm/verisilicon/vs_simple_enc.c
> > > > > @@ -0,0 +1,195 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +/*
> > > > > + * Copyright (C) 2020 VeriSilicon Holdings Co., Ltd.
> > > > > + */
> > > > > +#include <linux/component.h>
> > > > > +#include <linux/of_device.h>
> > > > > +#include <linux/module.h>
> > > > > +
> > > > > +#include <drm/drm_atomic_helper.h> #include <drm/drm_bridge.h>
> > > > > +#include <drm/drm_crtc_helper.h> #include <drm/drm_of.h>
> > > > > +#include <linux/regmap.h> #include <linux/media-bus-format.h>
> > > > > +#include <linux/mfd/syscon.h>
> > > > > +
> > > > > +#include "vs_crtc.h"
> > > > > +#include "vs_simple_enc.h"
> > > > > +
> > > > > +static const struct simple_encoder_priv dsi_priv = {
> > > >
> > > > Please use proper prefix for all the struct and function names.
> > > > vs_simple_encoder sounds better. Or vs_dsi_encoder.
> > > >
> > > > > + .encoder_type = DRM_MODE_ENCODER_DSI };
> > > > > +
> > > > > +static inline struct simple_encoder *to_simple_encoder(struct
> > > > > +drm_encoder *enc) {
> > > > > + return container_of(enc, struct simple_encoder,
> > > > > +encoder); }
> > > > > +
> > > > > +static int encoder_parse_dt(struct device *dev) {
> > > > > + struct simple_encoder *simple = dev_get_drvdata(dev);
> > > > > + unsigned int args[2];
> > > > > +
> > > > > + simple->dss_regmap =
> > > syscon_regmap_lookup_by_phandle_args(dev->of_node,
> > > > > +
> > > "starfive,syscon",
> > > > > +
> > > 2,
> > > > > + args);
> > > > > +
> > > > > + if (IS_ERR(simple->dss_regmap)) {
> > > > > + return dev_err_probe(dev,
> > > PTR_ERR(simple->dss_regmap),
> > > > > + "getting the regmap
> > > failed\n");
> > > > > + }
> > > > > +
> > > > > + simple->offset = args[0];
> > > > > + simple->mask = args[1];
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +void encoder_atomic_enable(struct drm_encoder *encoder,
> > > > > + struct drm_atomic_state *state) {
> > > > > + struct simple_encoder *simple =
> > > > > +to_simple_encoder(encoder);
> > > > > +
> > > > > + regmap_update_bits(simple->dss_regmap, simple->offset,
> > > simple->mask,
> > > > > + simple->mask); }
> > > > > +
> > > > > +int encoder_atomic_check(struct drm_encoder *encoder,
> > > > > + struct drm_crtc_state *crtc_state,
> > > > > + struct drm_connector_state *conn_state)
> {
> > > > > + struct vs_crtc_state *vs_crtc_state =
> > > to_vs_crtc_state(crtc_state);
> > > > > + struct drm_connector *connector = conn_state->connector;
> > > > > + int ret = 0;
> > > > > +
> > > > > + struct drm_bridge *first_bridge =
> > > drm_bridge_chain_get_first_bridge(encoder);
> > > > > + struct drm_bridge_state *bridge_state =
> > > > > + ERR_PTR(-EINVAL);
> > > > > +
> > > > > + vs_crtc_state->encoder_type = encoder->encoder_type;
> > > > > +
> > > > > + if (first_bridge &&
> first_bridge->funcs->atomic_duplicate_state)
> > > > > + bridge_state =
> > > > > + drm_atomic_get_bridge_state(crtc_state->state, first_bridge);
> > > >
> > > > Please don't poke into others' playground. This should go into
> > > > your DSI bridge's atomic_check() instead.
> > >
> > > Hmm. And you can not use vs_crtc_state from your bridge. Actually
> > > this design makes me wonder, how does your hardware work? Is it
> > > possible to send the DSI commands to the panel?
> > >
> > Hi Dmitry:
> > 1、 This fun is used to check the media bus format whether in support list , if
> not , this mode will be invalid,
> > I just used the bridge api to get the media bus format. (sync with
> > the input bridge(connector) bus format)
> > 2、 the bridge doesn't care the vs_crtc_state, just do their own
> drm_bridge_funcs ->atomic_check,
> > or it will be impossible to make this dsi encoder to fit dsi bridge driver.
> > 3、 hardware work :
> > encoder_atomic_check get the media bus format and update it to
> vs_crtc_state-> output_fmt,
> > during the crtc_enable , it will write the output_fmt to hardware .
> >
> > 4、 to send dsi command to panel , It is only relevant to dsi controllers
> and panel driver (bridge and panel )，
> > It does not involve the logic associated with encoder and crtc
>
> Do you have a pointer to your DSI bridge driver somewhere (a preliminary
> version is ok, if it's not ready yet for upstream submission). From the current
> design it seems that there is no need for a separate 'simple'
> encoder. Instead it might be easier/better to transform your DSI bridge driver
> into an encoder driver, especially as you are prety flexible in the component
> connections.
>
Hi Dmitry:
My SoC DSI bridge driver is cdns dsi and cdns dsi is already in the path:
/linux/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
Encoder_bind will attach the bridge to encoder
/* output port is port1*/
bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
......
return drm_bridge_attach(encoder, bridge, NULL, 0);
There are two types of bridges in the Linux mainline code:
Type 1: only contains bridge driver, for example
/linux/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
/linux/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
Type 2: Contains encoder + bridge:
/linux/drivers/gpu\drm/i2c/tda998x_drv.c
This simple encoder driver is used to match the Type 1 bridge.
Of course, if i adapt to Type 2, i only need to care the encoder possible crtc. Skip this encoder
Thanks
Keith
> >
> > https://elixir.bootlin.com/linux/v6.6.30/source/drivers/gpu/drm/mxsfb/
> > lcdif_kms.c#L457
> > Do the similar logic
>
> --
> With best wishes
> Dmitry
next prev parent reply other threads:[~2024-05-16 3:12 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-04 12:33 [v3 0/6] DRM driver for verisilicon Keith Zhao
2023-12-04 12:33 ` [v3 1/6] dt-bindings: display: Add yamls for JH7110 display system Keith Zhao
2023-12-05 6:59 ` Krzysztof Kozlowski
2023-12-06 12:50 ` Sui Jingfeng
2023-12-08 16:33 ` Rob Herring
2023-12-08 16:31 ` Rob Herring
2023-12-04 12:33 ` [v3 2/6] riscv: dts: starfive: jh7110: display subsystem Keith Zhao
2023-12-04 12:33 ` [v3 3/6] drm/vs: Register DRM device Keith Zhao
2023-12-04 13:30 ` Philipp Zabel
2023-12-11 9:00 ` Keith Zhao
2023-12-11 9:17 ` mripard
2023-12-05 11:33 ` Dmitry Baryshkov
2023-12-04 12:33 ` [v3 4/6] drm/vs: Add KMS crtc&plane Keith Zhao
2023-12-05 12:48 ` Dmitry Baryshkov
2023-12-06 8:55 ` Maxime Ripard
2023-12-06 12:53 ` Keith Zhao
2024-01-31 8:57 ` 回复: " Keith Zhao
2024-01-31 16:30 ` Maxime Ripard
2024-01-31 9:33 ` Keith Zhao
2024-01-31 13:23 ` Maxime Ripard
[not found] ` <NTZPR01MB1050C500508E29152E845511EE43A@NTZPR01MB1050.CHNPR01.prod.partner.outlook.cn>
2024-02-09 15:37 ` Maxime Ripard
2023-12-07 8:41 ` Icenowy Zheng
2023-12-07 11:31 ` Keith Zhao
2023-12-07 15:29 ` Icenowy Zheng
2023-12-27 3:57 ` Icenowy Zheng
2023-12-04 12:33 ` [v3 5/6] drm/vs: Add hdmi driver Keith Zhao
2023-12-05 13:02 ` Dmitry Baryshkov
2023-12-06 9:04 ` Maxime Ripard
2023-12-06 12:02 ` Keith Zhao
2023-12-06 12:56 ` Maxime Ripard
2023-12-06 14:11 ` Keith Zhao
2023-12-07 9:02 ` Andy Yan
2023-12-07 10:48 ` Keith Zhao
2023-12-08 0:37 ` Andy Yan
2023-12-08 3:00 ` Keith Zhao
2023-12-08 3:23 ` Andy Yan
2023-12-08 9:14 ` Maxime Ripard
2023-12-11 10:24 ` Keith Zhao
2023-12-11 12:13 ` Andy Yan
2023-12-13 1:40 ` Keith Zhao
2023-12-14 2:51 ` Andy Yan
2023-12-11 17:34 ` Rob Herring
2023-12-04 12:33 ` [v3 6/6] drm/vs: simple encoder Keith Zhao
2023-12-05 13:14 ` Dmitry Baryshkov
2023-12-05 13:18 ` Dmitry Baryshkov
2024-05-15 10:07 ` Keith Zhao
2024-05-15 15:17 ` Dmitry Baryshkov
2024-05-16 2:57 ` Keith Zhao [this message]
2023-12-05 6:55 ` [v3 0/6] DRM driver for verisilicon Krzysztof Kozlowski
2023-12-05 10:27 ` Sui Jingfeng
2023-12-05 11:59 ` Sui Jingfeng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=NTZPR01MB1050BDEAB3E0C1AF28F04FEDEEED2@NTZPR01MB1050.CHNPR01.prod.partner.outlook.cn \
--to=keith.zhao@starfivetech.com \
--cc=aou@eecs.berkeley.edu \
--cc=changhuang.liang@starfivetech.com \
--cc=devicetree@vger.kernel.org \
--cc=dmitry.baryshkov@linaro.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=jack.zhu@starfivetech.com \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=mripard@kernel.org \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=shengyang.chen@starfivetech.com \
--cc=suijingfeng@loongson.cn \
--cc=tzimmermann@suse.de \
--cc=william.qiu@starfivetech.com \
--cc=xingyu.wu@starfivetech.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).