Re: [PATCH 03/15] drm/dsi: Add mipi_dsi_set_maximum_return_packet_size() helper

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2762 bytes --]

On Mon, Oct 13, 2014 at 04:10:31PM +0200, Andrzej Hajda wrote:
> On 10/13/2014 12:16 PM, Thierry Reding wrote:
> > From: YoungJun Cho <yj44.cho@samsung.com>
> > 
> > This function can be used to set the maximum return packet size for a
> > MIPI DSI peripheral.
> > 
> > Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
> > [treding: endianess, kerneldoc, return value]
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/gpu/drm/drm_mipi_dsi.c | 30 ++++++++++++++++++++++++++++++
> >  include/drm/drm_mipi_dsi.h     |  2 ++
> >  2 files changed, 32 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> > index 1702ffd07986..27fc6dac5e4a 100644
> > --- a/drivers/gpu/drm/drm_mipi_dsi.c
> > +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> > @@ -198,6 +198,36 @@ int mipi_dsi_detach(struct mipi_dsi_device *dsi)
> >  }
> >  EXPORT_SYMBOL(mipi_dsi_detach);
> >  
> > +/*
> > + * mipi_dsi_set_maximum_return_packet_size() - specify the maximum size of the
> > + *    the payload in a long packet transmitted from the peripheral back to the
> > + *    host processor
> > + * @dsi: DSI peripheral device
> > + * @value: the maximum size of the payload
> > + *
> > + * Return: 0 on success or a negative error code on failure.
> > + */
> > +int mipi_dsi_set_maximum_return_packet_size(struct mipi_dsi_device *dsi,
> > +					    u16 value)
> > +{
> > +	u8 tx[2] = { value & 0xff, value >> 8 };
> > +	struct mipi_dsi_msg msg;
> > +	ssize_t err;
> > +
> > +	memset(&msg, 0, sizeof(msg));
> > +	msg.channel = dsi->channel;
> > +	msg.type = MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE;
> > +	msg.tx_len = sizeof(tx);
> > +	msg.tx_buf = tx;
> 
> 
> An alternative (without tx variable) could be:
> 
> 	__cpu_to_le16s(&value);
> 	msg.tx_len = sizeof(value);
> 	msg.tx_buf = &value;
> 
> But it is just pico-optimization.
> 
> 
> > +
> > +	err = dsi->host->ops->transfer(dsi->host, &msg);
> 
> I guess the sequence:
> 	if (dsi->mode_flags & MIPI_DSI_MODE_LPM)
> 		msg.flags = MIPI_DSI_MSG_USE_LPM;
> 
> should be before this callback.
> 
> The same should be in all other helpers calling ops->transfer,
> so I guess it may be good to move it all to separate function, for
> example sth like:
> ... dsi_transfer(dsi, msg)
> {
> 	if (!ops || !ops->transfer)
> 		return -NOSYS;
> 
> 	if (dsi->mode_flags & MIPI_DSI_MODE_LPM)
> 		msg.flags = MIPI_DSI_MSG_USE_LPM;
> 
> 
> 	return ops->transfer(dsi->host, msg);
> }

Good idea. It'd be slightly better if we could check for valid ops
earlier, but I don't think it's really worth it. I'll add a helper that
performs these common checks and rebase the other patches on top of it.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel