>
> Just a buch of drive-by comments while browsing the code.
> In general code looks good, especialyl for a v1.
>
> There is a few places that triggers warnings with checkpatch --strict
> Most looks like things that should be fixed.
>
>
Thanks Sam for the review. Will take care of the suggestions in next
iteration.
Response inlined below:
> > +struct pipe_msg_hdr {
> > + u32 type;
> > + u32 size; /* size of message after this field */
> > +} __packed;
> > +
> > +struct hvd_screen_info {
> > + u16 width;
> > + u16 height;
> > +} __packed;
> > +
> > +struct synthvid_msg_hdr {
> > + u32 type;
> > + u32 size; /* size of this header + payload after this field*/
> Add space before closing "*/"
>
> I wonder what is the difference between what is considered a message
> and
> what is considered payload in the above comments.
> Maybe that just because I do not know this stuff at all and the
> comment
> can be ignored.
message = struct pipe_msg_hdr + struct synthvid_msg_hdr + payload
Will try to make it more clear.
>
> > +} __packed;
> > +
> > +struct synthvid_version_req {
> > + u32 version;
> > +} __packed;
> > +
> > +struct synthvid_version_resp {
> > + u32 version;
> > + u8 is_accepted;
> > + u8 max_video_outputs;
> > +} __packed;
> > +
> > +struct synthvid_vram_location {
> > + u64 user_ctx;
> > + u8 is_vram_gpa_specified;
> > + u64 vram_gpa;
> > +} __packed;
> Not an alignmnet friendly layout - but I guess the layout is fixed.
> Same goes in otther places.
Yes nothing can be done for this.
>
> > +static int synthvid_update_situation(struct hv_device *hdev, u8
> > active, u32 bpp,
> > + u32 w, u32 h, u32 pitch)
> > +{
> > + struct synthvid_msg msg;
>
> Sometimes synthvid_msg is hv->init_buf.
> Sometimes a local variable.
> I wonder why there is a difference.
When a reply is expected, hv->init_buf should be used, though I haven't
verified this. Just kept the same logic as in framebuffer driver.
