From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark yao Subject: Re: [PATCH v5 1/3] drm/rockchip: Add basic drm driver Date: Thu, 25 Sep 2014 20:11:39 +0800 Message-ID: <5424067B.7090908@rock-chips.com> References: <1411524672-14524-1-git-send-email-mark.yao@rock-chips.com> <1411524746-14635-1-git-send-email-mark.yao@rock-chips.com> <5423D951.4030703@rock-chips.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1630992258==" Return-path: In-Reply-To: <5423D951.4030703@rock-chips.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Kurtz Cc: Mark Rutland , =?UTF-8?B?SGVpa28gU3TDvGJuZXI=?= , linux-doc@vger.kernel.org, kever.yang@rock-chips.com, dri-devel , "linux-kernel@vger.kernel.org" , xjq@rock-chips.com, zyw@rock-chips.com, linux-api@vger.kernel.org, jeff chen , linux-rockchip@lists.infradead.org, kfx@rock-chips.com, Grant Likely , wxt@rock-chips.com, huangtao@rock-chips.com, devicetree@vger.kernel.org, Pawel Moll , Ian Campbell , yxj@rock-chips.com, Eddie Cai , Rob Herring , =?UTF-8?B?U3TDqXBoYW5lIE1hcmNoZXNpbg==?= , simon xue , xw@rock-chips.com, Greg Kroah-Hartman , Randy Dunlap List-Id: linux-api@vger.kernel.org This is a multi-part message in MIME format. --===============1630992258== Content-Type: multipart/alternative; boundary="------------020802010800050801080108" This is a multi-part message in MIME format. --------------020802010800050801080108 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable On 2014=E5=B9=B409=E6=9C=8825=E6=97=A5 16:58, Mark yao wrote: > On 2014=E5=B9=B409=E6=9C=8825=E6=97=A5 00:20, Daniel Kurtz wrote: >> Hi Mark, >> >> Please review comments inline... >> >> On Wed, Sep 24, 2014 at 10:12 AM, Mark yao w= rote: >> To match the enum name, use ROCKCHIP_OUTPUT_TYPE_*. >> Also, no need to explicitly set the first one to 0. >> However, see below. I don't think we to modify the drm_display_mode >> to include an output type. > but vop devices need know the connector type, connector enable=20 > register is in vop. > can I do that like under to get connector type for crtc? > > static int rockchip_get_connector_type(struct drm_crtc *crtc) > { > struct drm_device *dev =3D crtc->dev; > struct drm_connector * connector; > > list_for_each_entry(connector,=20 > &dev->mode_config.connector_list, head) { > if (!connector->encoder) > continue; > /* > * one crtc only has one connector in my case, so just find=20 > the first connector at list. > */ > if (connector->encoder->crtc =3D=3D crtc) > break; > } > > if (!connector) > return -EINVAL; > > return connector->connector_type; > }=20 Oh, sorry, forgot to drop this comment, for connector type problem, I try to new a help function for encoder to=20 call as Daniel advices. >>> >>> +#define to_rockchip_plane(x) container_of(x, struct rockchip_plane, = base) >>> + >>> +struct rockchip_plane { >>> + int id; >>> + struct drm_plane base; >>> + const struct vop_win *win; >>> + struct vop_context *ctx; >> Isn't ctx just: to_vop_ctx(base->crtc) >> > OK. we can use to_vop_ctx(base->crtc) to get ctx.=20 I have do a test to use "to_vop_ctx(base->crtc)", but found that=20 "base->crtc" maybe not init. for cursor plane, base->crtc always is NULL. and disable_plane will fail. maybe we can add "base->crtc =3D crtc" at update_plane, but it seems not = good. so I think still use "rockchip_plane->ctx" would be better. -Mark --------------020802010800050801080108 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
On 2014=E5=B9=B409=E6=9C=8825=E6=97=A5= 16:58, Mark yao wrote:
On 2014=E5=B9=B409=E6=9C=8825=E6=97=A5= 00:20, Daniel Kurtz wrote:
Hi Mark,

Please review comments inline...

On Wed, Sep 24, 2014 at 10:12 AM, Mark yao <=
;mark.yao@rock-chips.com> wrote:

To match the enum name, use ROCKCHIP_OUTPUT_TYPE_*=
.
Also, no need to explicitly set the first one to 0.
However, see below.  I don't think we to modify the drm_display_mode
to include an output type.
but vop devices need know the connector type, connector enable register is in vop.
can I do that like under to=C2=A0 get connector type for crtc?

=C2=A0=C2=A0=C2=A0 static int rockchip_get_connector_type(struct dr= m_crtc *crtc)
=C2=A0=C2=A0=C2=A0 {
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct drm_d= evice *dev =3D crtc->dev;
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct drm_c= onnector * connector;

=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 list_for_eac= h_entry(connector, &dev->mode_config.connector_list, head) {
=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (!connector-&g= t;encoder)
=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0 continue;
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /*
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 * one = crtc only has one connector in my case, so just find the first connector at list.
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 */
=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (connector->= ;encoder->crtc =3D=3D crtc)
=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0 break;
=C2=A0=C2=A0=C2=A0 }

=C2=A0=C2=A0=C2=A0 if (!connector)
=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 retur= n -EINVAL;

=C2=A0=C2=A0=C2=A0 return connector->connector_type;
}
Oh, sorry, forgot to drop this comment,
for connector type problem, I try to new a help function for encoder to call as Daniel advices.

+#define to_rockchip_plane(x) container_of(x, struct rockchip_plane, base=
)
+
+struct rockchip_plane {
+       int id;
+       struct drm_plane base;
+       const struct vop_win *win;
+       struct vop_context *ctx;

Isn't ctx just: to_vop_ctx(base->crtc)
OK. we can use to_vop_ctx(base->crtc) to get ctx.
I have do a test to use "to_vop_ctx(base->crtc)", but found that "base->crtc" maybe not init.
for cursor plane, base->crtc always is NULL. and disable_plane will fail.
maybe we can add "base->crtc =3D crtc" at update_plane, but it seems not good.
so I think still use "rockchip_plane->ctx" would be better.

-Mark
--------------020802010800050801080108-- --===============1630992258== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel --===============1630992258==--